diff --git a/app/src/api/controllers/editHistoryController.ts b/app/src/api/controllers/editHistoryController.ts index b959d685..15950558 100644 --- a/app/src/api/controllers/editHistoryController.ts +++ b/app/src/api/controllers/editHistoryController.ts @@ -2,7 +2,7 @@ import express from 'express'; import { ApiParamError, ApiUserError } from '../errors/api'; import { ArgumentError } from '../errors/general'; -import { checkRegexParam, parsePositiveIntParam, processParam } from '../parameters'; +import { checkRegexParam, parsePositiveIntParam, processParam, parseBooleanParam } from '../parameters'; import asyncController from "../routes/asyncController"; import * as editHistoryService from '../services/editHistory'; @@ -12,13 +12,14 @@ const getGlobalEditHistory = asyncController(async (req: express.Request, res: e const afterId: string = processParam(req.query, 'after_id', x => checkRegexParam(x, revisionIdRegex)); const beforeId: string = processParam(req.query, 'before_id', x => checkRegexParam(x, revisionIdRegex)); const count: number = processParam(req.query, 'count', parsePositiveIntParam); + const filterDeletions: boolean = processParam(req.query, 'deletions', parseBooleanParam); if(afterId != undefined && beforeId != undefined) { throw new ApiUserError('Cannot specify both after_id and before_id parameters'); } try { - const result = await editHistoryService.getGlobalEditHistory(beforeId, afterId, count); + const result = await editHistoryService.getGlobalEditHistory(beforeId, afterId, count, filterDeletions); res.send(result); } catch(error) { if(error instanceof ArgumentError && error.argumentName === 'count') { diff --git a/app/src/api/dataAccess/__mocks__/editHistory.ts b/app/src/api/dataAccess/__mocks__/editHistory.ts index b4636864..735e71c1 100644 --- a/app/src/api/dataAccess/__mocks__/editHistory.ts +++ b/app/src/api/dataAccess/__mocks__/editHistory.ts @@ -17,18 +17,27 @@ mockEditHistory.__setHistory = function(mockHistoryData: EditHistoryEntry[]) { mockData = mockHistoryData.sort(numDesc(x => BigInt(x.revision_id))); }; -mockEditHistory.getHistoryAfterId = function(id: string, count: number): Promise { +function hasDeletions(json) { + return Object.values(json).some(x => x === null); +} + +mockEditHistory.getHistoryAfterId = function(id: string, count: number, filterDeletions: boolean): Promise { return Promise.resolve( mockData - .filter(x => BigInt(x.revision_id) > BigInt(id)) + .filter(x => BigInt(x.revision_id) > BigInt(id) && (!filterDeletions || hasDeletions(x.forward_patch))) .sort(numAsc(x => BigInt(x.revision_id))) .slice(0, count) .sort(numDesc(x => BigInt(x.revision_id))) ); }; -mockEditHistory.getHistoryBeforeId = function(id: string, count: number): Promise { +mockEditHistory.getHistoryBeforeId = function(id: string, count: number, filterDeletions: boolean): Promise { let filteredData = id == undefined ? mockData : mockData.filter(x => BigInt(x.revision_id) < BigInt(id)); + + if(filterDeletions) { + filteredData = filteredData.filter(x => hasDeletions(x.forward_patch)); + } + return Promise.resolve( filteredData .slice(0, count) diff --git a/app/src/api/dataAccess/editHistory.ts b/app/src/api/dataAccess/editHistory.ts index c500f293..eba1cfa1 100644 --- a/app/src/api/dataAccess/editHistory.ts +++ b/app/src/api/dataAccess/editHistory.ts @@ -13,7 +13,11 @@ const baseQuery = ` FROM logs JOIN users ON logs.user_id = users.user_id`; -export function getHistoryAfterId(id: string, count: number): Promise { +const deletionCondition = ` + EXISTS (SELECT * from jsonb_each(forward_patch) WHERE value = 'null' ) +`; + +export function getHistoryAfterId(id: string, count: number, filterDeletions: boolean = false): Promise { /** * SQL with lower time bound specified (records after ID). * The outer SELECT is so that final results are sorted by descending ID @@ -25,6 +29,7 @@ export function getHistoryAfterId(id: string, count: number): Promise $1 + ${filterDeletions ? 'AND ' + deletionCondition : ''} ORDER BY revision_id ASC LIMIT $2 ) AS result_asc ORDER BY revision_id DESC`, @@ -35,12 +40,13 @@ export function getHistoryAfterId(id: string, count: number): Promise { +export function getHistoryBeforeId(id: string, count: number, filterDeletions: boolean = false): Promise { try { if(id == undefined) { return db.any(` ${baseQuery} + ${filterDeletions ? 'WHERE' + deletionCondition : ''} ORDER BY revision_id DESC LIMIT $1 `, [count]); @@ -50,6 +56,7 @@ export function getHistoryBeforeId(id: string, count: number): Promise { }); }); + + describe('Filtering deletions', () => { + + it('Should return only records with deletions when requested', async () => { + const mockData = generateHistory(10); + mockData[0].forward_patch = { 'test_field': null}; + mockData[0].reverse_patch = { 'test_field': 'test'}; + + mockData[3].forward_patch = { 'test_field': null}; + mockData[3].reverse_patch = { 'test_field': 'test'}; + + mockData[4].forward_patch = { 'test_field': 'test2'}; + mockData[4].reverse_patch = { 'test_field': 'test1'}; + mockedEditHistoryData.__setHistory(mockData); + + const result = await getGlobalEditHistory(null, null, 10, true); + + expect(result.history.map(x => x.revision_id)).toEqual(['103', '100']); + }); + }); }); diff --git a/app/src/api/services/editHistory.ts b/app/src/api/services/editHistory.ts index 81b4c899..b6d166f2 100644 --- a/app/src/api/services/editHistory.ts +++ b/app/src/api/services/editHistory.ts @@ -3,7 +3,12 @@ import { decBigInt, incBigInt } from '../../helpers'; import { getHistoryAfterId, getHistoryBeforeId, getIdNewerThan, getIdOlderThan } from '../dataAccess/editHistory'; import { ArgumentError } from '../errors/general'; -async function getGlobalEditHistory(beforeId?: string, afterId?: string, count: number = 100) { +async function getGlobalEditHistory( + beforeId?: string, + afterId?: string, + count: number = 100, + filterDeletions: boolean = false +) { if(count <= 0) throw new ArgumentError('cannot request less than 1 history record', 'count'); if(count > 100) count = 100; @@ -11,9 +16,9 @@ async function getGlobalEditHistory(beforeId?: string, afterId?: string, count: let editHistoryRecords: EditHistoryEntry[]; if(afterId != undefined) { - editHistoryRecords = await getHistoryAfterId(afterId, count); + editHistoryRecords = await getHistoryAfterId(afterId, count, filterDeletions); } else { - editHistoryRecords = await getHistoryBeforeId(beforeId, count); + editHistoryRecords = await getHistoryBeforeId(beforeId, count, filterDeletions); } const currentBatchMaxId = editHistoryRecords[0]?.revision_id ?? decBigInt(beforeId); diff --git a/app/src/frontend/pages/changes.tsx b/app/src/frontend/pages/changes.tsx index f12aceb3..850be59a 100644 --- a/app/src/frontend/pages/changes.tsx +++ b/app/src/frontend/pages/changes.tsx @@ -17,8 +17,26 @@ interface PagingInfo { const recordsPerPage = 20; +function getNavigationUrl(options: { + afterId?: string, + beforeId?: string, + deletions?: string +}) { + let query = '?'; + let params = []; + if(options == undefined) return query; + + if(options.afterId != undefined) params.push(`after_id=${options.afterId}`); + if(options.beforeId != undefined) params.push(`before_id=${options.beforeId}`); + if(options.deletions != undefined) params.push(`deletions=${options.deletions}`); + + return query + params.join('&'); +} + const ChangesPage = (props: RouteComponentProps) => { - const { after_id, before_id } = parse(props.location.search); + let { after_id, before_id, deletions } = parse(props.location.search); + + if(deletions instanceof Array) deletions = deletions[0]; const [history, setHistory] = useState(); const [paging, setPaging] = useState(); @@ -37,6 +55,11 @@ const ChangesPage = (props: RouteComponentProps) => { if (before_id) { url = `${url}&before_id=${before_id}`; } + + if(deletions) { + url = `${url}&deletions=${deletions}`; + } + try { const {history, paging, error} = await apiGet(url); @@ -60,16 +83,30 @@ const ChangesPage = (props: RouteComponentProps) => {

Global edit history

- { paging?.id_for_newer_query && - Show more recent changes + Show more recent changes } { (after_id || before_id) && - Show latest changes + Show latest changes }
    + { + deletions === 'true' && + + } { error && @@ -97,7 +134,10 @@ const ChangesPage = (props: RouteComponentProps) => {
{ paging?.id_for_older_query && - Show older changes + Show older changes }