Update edit history tests and implementation

app/src/api/services/__tests__/editHistory.test.ts

@@ -1,5 +1,6 @@
 import { EditHistoryEntry } from '../../../frontend/models/edit-history-entry';
 import * as editHistoryData from '../../dataAccess/editHistory'; // manually mocked
+import { UserInputError } from '../../errors';
 import { getGlobalEditHistory } from '../editHistory';
 
 jest.mock('../../dataAccess/editHistory');
@@ -19,10 +20,16 @@ function generateHistory(n: number, firstId: number = 100) {
 
 describe('getGlobalEditHistory()', () => {
     
-    beforeEach(() => {
+    beforeEach(() => mockedEditHistoryData.__setHistory(generateHistory(20)));
-        mockedEditHistoryData.__setHistory(
+
-            generateHistory(20)
+    afterEach(() => jest.clearAllMocks());
-        );
+
+    it.each([
+        [null, null],
+        ['100', null],
+        [null, '100']
+    ])('Should error when requesting non-positive number of records', (beforeId: string, afterId: string) => {
+        expect(getGlobalEditHistory(beforeId, afterId, 0)).rejects.toBeInstanceOf(UserInputError);
     });
     
     describe('getting history before a point', () => {
@@ -37,7 +44,9 @@ describe('getGlobalEditHistory()', () => {
             [
                 [null, 3, ['119', '118', '117']],
                 [null, 6, ['119', '118', '117', '116', '115', '114']],
-                ['118', 0, []],
+                ['118', 1, ['117']],
+                ['104', 10, ['103', '102','101', '100']],
+                ['100', 2, []]
             ]
         )('should return the N records before the specified ID in descending order [beforeId: %p, count: %p]', async (
             beforeId: string, count: number, ids: string[]
@@ -48,43 +57,39 @@ describe('getGlobalEditHistory()', () => {
         });
 
         it.each([
-            [null, 4, true, false],
+            [null, 4, null],
-            [null, 10, true, false],
+            [null, 10, null],
-            [null, 20, false, false],
+            [null, 20, null],
-            [null, 30, false, false],
+            [null, 30, null],
-            ['50', 10, false, true],
+            ['50', 10, '99'],
-            ['100', 10, false, true],
+            ['100', 10, '99'],
-            ['105', 2, true, true],
+            ['130', 10, null],
-        ])('should indicate if there are any newer or older records [beforeId: %p, count: %p]', async (
+            ['105', 2, '104'],
-            beforeId: string, count: number, hasOlder: boolean, hasNewer: boolean
+            ['120', 20, null],
+        ])('should detect if there are any newer records left [beforeId: %p, count: %p]', async (
+            beforeId: string, count: number, idForNewerQuery: string
         ) => {
             const result = await getGlobalEditHistory(beforeId, null, count);
 
-            expect(result.paging.has_older).toBe(hasOlder);
+            expect(result.paging.id_for_newer_query).toBe(idForNewerQuery);
-            expect(result.paging.has_newer).toBe(hasNewer);
         });
 
+        it.each([
+            [null, 4, '116'],
+            [null, 10, '110'],
+            [null, 20, null],
+            [null, 30, null],
+            ['50', 10, null],
+            ['100', 10, null],
+            ['130', 10, '110'],
+            ['105', 2, '103'],
+            ['120', 20, null],
+        ])('should detect if there are any older records left [beforeId: %p, count: %p]', async (
+            beforeId: string, count: number, idForOlderQuery: string
+        ) => {
+            const result = await getGlobalEditHistory(beforeId, null, count);
 
-        it('should not return more than 100 entries', async () => {
+            expect(result.paging.id_for_older_query).toBe(idForOlderQuery);
-            mockedEditHistoryData.__setHistory(
-                generateHistory(200)
-            );
-
-            const result = await getGlobalEditHistory(null, null, 200);
-
-            expect(result.paging.has_older).toBeTruthy();
-            expect(result.history.length).toBe(100);
-        });
-
-        it('should default to 100 entries', async () => {
-            mockedEditHistoryData.__setHistory(
-                generateHistory(200)
-            );
-
-            const result = await getGlobalEditHistory(null, null, 200);
-
-            expect(result.paging.has_older).toBeTruthy();
-            expect(result.history.length).toBe(100);
         });
     });
 
@@ -104,17 +109,66 @@ describe('getGlobalEditHistory()', () => {
         });
 
         it.each([
-            ['99', 10, false, true],
+            ['99', 10, '109'],
-            ['110', 5, true, true],
+            ['110', 5, '115'],
-            ['119', 20, true, false],
+            ['119', 20, null],
-            ['99', 20, false, false]
+            ['99', 20, null],
-        ])('should indicate if there are any newer or older records [afterId: %p, count: %p]', async (
+        ])('should detect if there are any newer records left [afterId: %p, count: %p]', async (
-            afterId: string, count: number, hasOlder:boolean, hasNewer: boolean
+            afterId: string, count: number, idForNewerQuery: string
         ) => {
             const result = await getGlobalEditHistory(null, afterId, count);
 
-            expect(result.paging.has_older).toBe(hasOlder);
+            expect(result.paging.id_for_newer_query).toBe(idForNewerQuery);
-            expect(result.paging.has_newer).toBe(hasNewer);
         });
+
+        it.each([
+            ['99', 10, null],
+            ['110', 5, '111'],
+            ['119', 20, '120'],
+            ['99', 20, null],
+        ])('should detect if there are any older records left [afterId: %p, count: %p]', async (
+            afterId: string, count: number, idForOlderQuery: string
+        ) => {
+            const result = await getGlobalEditHistory(null, afterId, count);
+
+            expect(result.paging.id_for_older_query).toBe(idForOlderQuery);
+        });
+        
+    });
+
+    describe('result count limit', () => {
+
+        it.each([
+            [null, null],
+            [null, '100'],
+            ['300', null]
+        ])('should not return more than 100 entries (beforeId: %p, afterId: %p)', async (
+            beforeId: string, afterId: string
+        ) => {
+            mockedEditHistoryData.__setHistory(
+                generateHistory(200)
+            );
+
+            const result = await getGlobalEditHistory(beforeId, afterId, 200);
+
+            expect(result.history.length).toBe(100);
+        });
+
+        it.each([
+            [null, null],
+            [null, '100'],
+            ['300', null]
+        ])('should default to 100 entries', async (
+            beforeId: string, afterId: string
+        ) => {
+            mockedEditHistoryData.__setHistory(
+                generateHistory(200)
+            );
+
+            const result = await getGlobalEditHistory(beforeId, afterId);
+
+            expect(result.history.length).toBe(100);
+        });
+
     });
 });

app/src/api/services/editHistory.ts

@@ -1,7 +1,10 @@
 import { EditHistoryEntry } from '../../frontend/models/edit-history-entry';
+import { decBigInt, incBigInt } from '../../helpers';
 import { getHistoryAfterId, getHistoryBeforeId, getIdNewerThan, getIdOlderThan } from '../dataAccess/editHistory';
+import { UserInputError } from '../errors';
 
 async function getGlobalEditHistory(beforeId?: string, afterId?: string, count: number = 100) {
+    if(count <= 0) throw new UserInputError('cannot request less than 1 history record');
     if(count > 100) count = 100;
 
     // limited set of records. Expected to be already ordered from newest to oldest
@@ -13,22 +16,20 @@ async function getGlobalEditHistory(beforeId?: string, afterId?: string, count:
         editHistoryRecords = await getHistoryBeforeId(beforeId, count);
     }
 
-    const currentBatchNewerBound = editHistoryRecords[0]?.revision_id ?? beforeId;
+    const currentBatchMaxId = editHistoryRecords[0]?.revision_id ?? decBigInt(beforeId);
-    const newer = currentBatchNewerBound && await getIdNewerThan(currentBatchNewerBound);
+    const newer = currentBatchMaxId && await getIdNewerThan(currentBatchMaxId);
 
-    const currentBatchOlderBound = editHistoryRecords[editHistoryRecords.length-1]?.revision_id ?? afterId;
+    const currentBatchMinId = editHistoryRecords[editHistoryRecords.length-1]?.revision_id ?? incBigInt(afterId);
-    const older = currentBatchOlderBound && await getIdOlderThan(currentBatchOlderBound);
+    const older = currentBatchMinId && await getIdOlderThan(currentBatchMinId);
 
-    const hasNewer = newer != undefined;
+    const idForOlderQuery = older != undefined ? incBigInt(older) : null;
-    const hasOlder = older != undefined;
+    const idForNewerQuery = newer != undefined ? decBigInt(newer) : null;
 
     return {
         history: editHistoryRecords,
         paging: {
-            has_newer: hasNewer,
+            id_for_newer_query: idForNewerQuery,
-            id_for_newer_query: null,
+            id_for_older_query: idForOlderQuery
-            has_older: hasOlder,
-            id_for_older_query: null
         }
     };
 }

app/src/helpers.ts

@@ -46,3 +46,15 @@ export function numAsc<T, V extends number | bigint>(accessor: AccessorFunction<
 export function numDesc<T, V extends number | bigint>(accessor: AccessorFunction<T, V>): CompareFunction<T> {
     return (a: T, b: T) => Number(accessor(b) - accessor(a));
 }
+
+/** 
+ * As of Jan 2020, bigint literals e.g. 1n can only be used with TS target esnext
+ * which then doesn't transpile the null conditional/coalescing operators and breaks on Node 12
+ * So BigInt(1) needs to be used here
+ * */ 
+export function incBigInt(bigStr: string): string {
+    return bigStr == undefined ? bigStr : String(BigInt(bigStr) + BigInt(1));
+}
+export function decBigInt(bigStr: string): string {
+    return bigStr == undefined ? bigStr : String(BigInt(bigStr) - BigInt(1));
+}