diff --git a/packages/api-client/src/constants.ts b/packages/api-client/src/constants.ts index 26cd72ddc7..badcb7794a 100644 --- a/packages/api-client/src/constants.ts +++ b/packages/api-client/src/constants.ts @@ -16,9 +16,9 @@ const productionSubdomains = [ 'psss', 'psss-api', 'report-api', + 'sync-api', 'entity-api', 'data-table-api', - 'sync-api', 'www', 'api', // this must go last in the array, otherwise it will be detected before e.g. admin-api ]; diff --git a/packages/central-server/src/permissions/assertions.js b/packages/central-server/src/permissions/assertions.js index a88260755b..ccd56faacd 100644 --- a/packages/central-server/src/permissions/assertions.js +++ b/packages/central-server/src/permissions/assertions.js @@ -1,24 +1,24 @@ +import { TUPAIA_ADMIN_PANEL_PERMISSION_GROUP } from '@tupaia/constants'; import { ensure } from '@tupaia/tsutils'; import { PermissionsError } from '@tupaia/utils'; -import { TUPAIA_ADMIN_PANEL_PERMISSION_GROUP } from './constants'; /* Re-export for backward compatibility. Prefer importing directly from @tupaia/access-policy. */ export { allowNoPermissions, + assertAdminPanelAccess, assertAllPermissions, assertAnyPermissions, + assertBESAdminAccess, + assertPermissionGroupAccess, + assertPermissionGroupsAccess, + assertVizBuilderAccess, hasBESAdminAccess, - hasVizBuilderAccess, hasPermissionGroupAccess, hasPermissionGroupsAccess, hasSomePermissionGroupsAccess, - assertBESAdminAccess, - assertVizBuilderAccess, hasTupaiaAdminPanelAccess, hasTupaiaAdminPanelAccessToCountry, - assertAdminPanelAccess, - assertPermissionGroupAccess, - assertPermissionGroupsAccess, + hasVizBuilderAccess, } from '@tupaia/access-policy'; /** diff --git a/packages/database/src/core/modelClasses/Entity.js b/packages/database/src/core/modelClasses/Entity.js index 1ced8bbc19..853db7a9d5 100644 --- a/packages/database/src/core/modelClasses/Entity.js +++ b/packages/database/src/core/modelClasses/Entity.js @@ -337,6 +337,10 @@ export class EntityRecord extends DatabaseRecord { export class EntityModel extends MaterializedViewLogDatabaseModel { static syncDirection = SyncDirections.BIDIRECTIONAL; + get excludedFieldsFromSync() { + return ['point', 'bounds', 'region', 'parent_id']; + } + get DatabaseRecordClass() { return EntityRecord; } @@ -707,9 +711,4 @@ export class EntityModel extends MaterializedViewLogDatabaseModel { groupBy: ['entity.id'], }; } - - sanitizeForClient = data => { - const { point, bounds, region, parent_id, ...sanitizedData } = data; - return sanitizedData; - }; } diff --git a/packages/database/src/core/modelClasses/SurveyResponse/leaderboard.js b/packages/database/src/core/modelClasses/SurveyResponse/leaderboard.js index 338a1f47ea..274205e408 100644 --- a/packages/database/src/core/modelClasses/SurveyResponse/leaderboard.js +++ b/packages/database/src/core/modelClasses/SurveyResponse/leaderboard.js @@ -32,7 +32,7 @@ export function getLeaderboardQuery(projectId = '') { return ` SELECT r.user_id, user_account.first_name, user_account.last_name, r.coconuts, r.pigs FROM ( - SELECT user_id, COUNT(*)::INT as coconuts, FLOOR(COUNT(*) / 100)::INT AS pigs + SELECT user_id, COUNT(*)::INT AS coconuts, FLOOR(COUNT(*) / 100)::INT AS pigs FROM survey_response JOIN survey ON survey.id=survey_id ${projectId ? 'WHERE survey.project_id = ?' : ''} diff --git a/packages/database/src/core/sync/buildSyncLookupSelect.js b/packages/database/src/core/sync/buildSyncLookupSelect.js index ebcdb2a235..545e4a9938 100644 --- a/packages/database/src/core/sync/buildSyncLookupSelect.js +++ b/packages/database/src/core/sync/buildSyncLookupSelect.js @@ -4,6 +4,7 @@ export async function buildSyncLookupSelect(model, columns = {}) { const attributes = Object.keys(await model.fetchSchema()); const { projectIds } = columns; const table = model.databaseRecord; + const excludedFields = [...(model.excludedFieldsFromSync || []), ...COLUMNS_EXCLUDED_FROM_SYNC]; return ` SELECT @@ -12,9 +13,7 @@ export async function buildSyncLookupSelect(model, columns = {}) { COALESCE(:updatedAtSyncTick, ${table}.updated_at_sync_tick), sync_device_tick.device_id, json_build_object( - ${attributes - .filter(a => !COLUMNS_EXCLUDED_FROM_SYNC.includes(a)) - .map(a => `'${a}', ${table}.${a}`)} + ${attributes.filter(a => !excludedFields.includes(a)).map(a => `'${a}', ${table}.${a}`)} ), ${projectIds || 'NULL'} `; diff --git a/packages/datatrak-web/jest.setup.js b/packages/datatrak-web/jest.setup.js index 1e5abff1ca..fd769bfe2a 100644 --- a/packages/datatrak-web/jest.setup.js +++ b/packages/datatrak-web/jest.setup.js @@ -16,13 +16,77 @@ Object.defineProperty(window, 'matchMedia', { })), }); -// TODO: Set up database for testing later +const mockModels = { + localSystemFact: { + get: jest.fn().mockResolvedValue('test-device-id'), + set: jest.fn().mockResolvedValue(undefined), + addProjectForSync: jest.fn(), + }, + closeDatabaseConnections: jest.fn(), +}; jest.mock('@tupaia/database', () => ({ migrate: jest.fn(), ModelRegistry: jest.fn().mockImplementation(() => ({})), })); -jest.mock('./src/database/DatatrakDatabase', () => ({ - DatatrakDatabase: jest.fn().mockImplementation(() => ({})), +jest.mock('./src/database/createDatabase', () => ({ + createDatabase: jest.fn().mockImplementation(() => ({ + models: { + localSystemFact: { + get: jest.fn().mockImplementation(arg => { + if (arg === 'deviceId') { + return 'test-device-id'; + } + return undefined; + }), + addProjectForSync: jest.fn(), + }, + }, + })), })); + +jest.mock('./src/api/CurrentUserContext', () => { + const actual = jest.requireActual('./src/api/CurrentUserContext'); + + return { + ...actual, + useCurrentUserContext: jest.fn(() => ({ + ...actual.useCurrentUserContext(), // Get the actual return value + accessPolicy: {}, // Override just this property + })), + }; +}); + +// TODO: Set up database for testing later +jest.mock('./src/api/DatabaseContext', () => { + const React = require('react'); + + return { + DatabaseContext: React.createContext({ + models: mockModels, + }), + DatabaseProvider: ({ children }) => children, + useDatabaseContext: () => ({ + models: mockModels, + }), + }; +}); + +jest.mock('./src/api/SyncContext', () => { + const React = require('react'); + + return { + SyncContext: React.createContext({ + clientSyncManager: { + triggerSync: jest.fn(), + }, + }), + SyncProvider: ({ children }) => children, + useSyncContext: () => ({ + clientSyncManager: { + triggerSync: jest.fn(), + }, + }), + }; +}); diff --git a/packages/datatrak-web/src/__tests__/__integration__/login.test.tsx b/packages/datatrak-web/src/__tests__/__integration__/login.test.tsx index fd20282bf9..9aa5c627c2 100644 --- a/packages/datatrak-web/src/__tests__/__integration__/login.test.tsx +++ b/packages/datatrak-web/src/__tests__/__integration__/login.test.tsx @@ -40,7 +40,7 @@ describe('Login', () => { renderPage('/login'); expect(await screen.findByRole('heading', { level: 2 })).toHaveTextContent('Log in'); await doLogin(); - server.use(mockUserRequest({ email: 'john@gmail.com' })); + server.use(mockUserRequest({ email: 'john@gmail.com', accessPolicy: [] })); expect(await screen.findByRole('heading', { level: 1 })).toHaveTextContent(/Select project/i); }); @@ -49,7 +49,7 @@ describe('Login', () => { renderPage('/login'); expect(await screen.findByRole('heading', { level: 2 })).toHaveTextContent('Log in'); - server.use(mockUserRequest({ email: 'john@gmail.com', projectId: 'foo' })); + server.use(mockUserRequest({ email: 'john@gmail.com', projectId: 'foo', accessPolicy: [] })); await doLogin(); await screen.findByText(/Select survey/i); @@ -59,7 +59,7 @@ describe('Login', () => { renderPage('/survey'); expect(await screen.findByRole('heading', { level: 2 })).toHaveTextContent('Log in'); - server.use(mockUserRequest({ email: 'john@gmail.com', projectId: 'foo' })); + server.use(mockUserRequest({ email: 'john@gmail.com', projectId: 'foo', accessPolicy: [] })); await doLogin(); expect(await screen.findByRole('heading', { level: 1 })).toHaveTextContent(/Select survey/i); diff --git a/packages/datatrak-web/src/__tests__/__integration__/survey.test.tsx b/packages/datatrak-web/src/__tests__/__integration__/survey.test.tsx index dfd7a01c1d..4e36eed920 100644 --- a/packages/datatrak-web/src/__tests__/__integration__/survey.test.tsx +++ b/packages/datatrak-web/src/__tests__/__integration__/survey.test.tsx @@ -9,7 +9,14 @@ import { handlers } from '../mocks/handlers'; const server = setupServer( ...handlers, rest.get('*/v1/getUser', (_, res, ctx) => { - return res(ctx.status(200), ctx.json({ name: 'John Smith', email: 'john@gmail.com' })); + return res( + ctx.status(200), + ctx.json({ + name: 'John Smith', + email: 'john@gmail.com', + id: '0'.repeat(24), + }), + ); }), rest.get('*/v1/*', (_, res, ctx) => { return res(ctx.status(200), ctx.json([])); diff --git a/packages/datatrak-web/src/__tests__/features/Survey/CopySurveyUrlButton.test.tsx b/packages/datatrak-web/src/__tests__/features/Survey/CopySurveyUrlButton.test.tsx index d3d2205b3e..67e554b0bb 100644 --- a/packages/datatrak-web/src/__tests__/features/Survey/CopySurveyUrlButton.test.tsx +++ b/packages/datatrak-web/src/__tests__/features/Survey/CopySurveyUrlButton.test.tsx @@ -9,7 +9,10 @@ import { handlers } from '../../mocks/handlers'; const server = setupServer( ...handlers, rest.get('*/v1/getUser', (_, res, ctx) => { - return res(ctx.status(200), ctx.json({ name: 'John Smith', email: 'john@gmail.com' })); + return res( + ctx.status(200), + ctx.json({ name: 'John Smith', email: 'john@gmail.com', id: '0'.repeat(24) }), + ); }), ); diff --git a/packages/datatrak-web/src/api/DatabaseContext.tsx b/packages/datatrak-web/src/api/DatabaseContext.tsx index 0479cc5e83..5f9597b2ea 100644 --- a/packages/datatrak-web/src/api/DatabaseContext.tsx +++ b/packages/datatrak-web/src/api/DatabaseContext.tsx @@ -15,12 +15,18 @@ export const DatabaseProvider = ({ children }: { children: Readonly(null); useEffect(() => { + let modelsInstance: DatatrakWebModelRegistry | null = null; const init = async () => { const { models } = await createDatabase(); + modelsInstance = models; setModels(models); }; init(); + + return () => { + modelsInstance?.closeDatabaseConnections(); + }; }, []); if (!models) { diff --git a/packages/datatrak-web/src/api/mutations/useEditUser.ts b/packages/datatrak-web/src/api/mutations/useEditUser.ts index b371be4680..de67dbc52f 100644 --- a/packages/datatrak-web/src/api/mutations/useEditUser.ts +++ b/packages/datatrak-web/src/api/mutations/useEditUser.ts @@ -38,12 +38,13 @@ export const useEditUser = (onSuccess?: () => void) => { await put('me', { data: updates }); }, { - onSuccess: async (_, variables) => { + onSuccess: (_, variables) => { queryClient.invalidateQueries(['getUser']); // If the user changes their project, we need to invalidate the entity descendants query so that recent entities are updated if they change back to the previous project without refreshing the page if (variables.projectId) { queryClient.invalidateQueries(['entityDescendants']); queryClient.invalidateQueries(['tasks']); + models.localSystemFact.addProjectForSync(variables.projectId); } onSuccess?.(); diff --git a/packages/datatrak-web/src/api/queries/useTask.ts b/packages/datatrak-web/src/api/queries/useTask.ts index 201a817ad3..145a7abab2 100644 --- a/packages/datatrak-web/src/api/queries/useTask.ts +++ b/packages/datatrak-web/src/api/queries/useTask.ts @@ -3,9 +3,9 @@ import { getTask } from '../../database'; import { DatatrakWebModelRegistry } from '../../types'; import { get } from '../api'; import { useIsOfflineFirst } from '../offlineFirst'; -import { ContextualQueryFunctionContext, useDatabaseQuery } from './useDatabaseQuery'; +import { useDatabaseQuery } from './useDatabaseQuery'; -export interface UseTaskLocalContext extends ContextualQueryFunctionContext { +export interface UseTaskLocalContext { models: DatatrakWebModelRegistry; taskId?: string; } diff --git a/packages/datatrak-web/src/routes/PrivateRoute.tsx b/packages/datatrak-web/src/routes/PrivateRoute.tsx index 081fb4aaa1..d884af28ff 100644 --- a/packages/datatrak-web/src/routes/PrivateRoute.tsx +++ b/packages/datatrak-web/src/routes/PrivateRoute.tsx @@ -1,4 +1,4 @@ -import React, { ReactElement } from 'react'; +import React, { ReactElement, useEffect } from 'react'; import { Navigate, Outlet, useLocation } from 'react-router-dom'; import { isFeatureEnabled } from '@tupaia/utils'; @@ -6,11 +6,22 @@ import { isFeatureEnabled } from '@tupaia/utils'; import { useCurrentUserContext } from '../api'; import { ADMIN_ONLY_ROUTES, ROUTES } from '../constants'; import { isWebApp } from '../utils'; +import { useDatabaseContext } from '../hooks/database'; // Reusable wrapper to handle redirecting to login if user is not logged in and the route is private export const PrivateRoute = ({ children }: { children?: ReactElement }): ReactElement => { const { isLoggedIn, hasAdminPanelAccess, hideWelcomeScreen, ...user } = useCurrentUserContext(); const { pathname, search } = useLocation(); + const { models } = useDatabaseContext(); + + useEffect(() => { + const addProjectForSync = async () => { + await models.localSystemFact.addProjectForSync(user.projectId); + }; + if (isLoggedIn && user.projectId) { + addProjectForSync(); + } + }, [models, isLoggedIn, user.projectId]); if (!isLoggedIn) { return ( diff --git a/packages/datatrak-web/src/sync/ClientSyncManager.ts b/packages/datatrak-web/src/sync/ClientSyncManager.ts index 3c5b86d0fc..c1e80db1a3 100644 --- a/packages/datatrak-web/src/sync/ClientSyncManager.ts +++ b/packages/datatrak-web/src/sync/ClientSyncManager.ts @@ -435,7 +435,7 @@ export class ClientSyncManager { pullProgressCallback(records.length); }; - const batchSize = 200; + const batchSize = 10000; await pullIncomingChanges(this.models, sessionId, batchSize, processStreamedDataFunction); this.setProgress(this.progressMaxByStage[SYNC_STAGES.PERSIST - 1], 'Saving changes...'); diff --git a/packages/server-utils/src/ScheduledTask.ts b/packages/server-utils/src/ScheduledTask.ts index d3c4b6e7b0..f9189d3c02 100644 --- a/packages/server-utils/src/ScheduledTask.ts +++ b/packages/server-utils/src/ScheduledTask.ts @@ -61,6 +61,8 @@ export class ScheduledTask { */ models: DatabaseInterface; + isRunning = false; + constructor(models: DatabaseInterface, name: string, schedule: string) { if (!name) { throw new Error(`ScheduledTask has no name`); @@ -84,7 +86,16 @@ export class ScheduledTask { async runTask() { this.start = Date.now(); + if (this.isRunning) { + const durationMs = Date.now() - this.start; + winston.info(`ScheduledTask: ${this.name}: Not running (previous task still running)`, { + durationMs, + }); + return false; + } + try { + this.isRunning = true; await this.models.wrapInTransaction(async (transactingModels: DatabaseInterface) => { // Acquire a database advisory lock for the transaction // Ensures no other server instance can execute its change handler at the same time @@ -101,6 +112,7 @@ export class ScheduledTask { return false; } finally { this.start = null; + this.isRunning = false; } } diff --git a/packages/superset-api/package.json b/packages/superset-api/package.json index da270e3b4d..01479506af 100644 --- a/packages/superset-api/package.json +++ b/packages/superset-api/package.json @@ -18,6 +18,7 @@ "test": "yarn package:test" }, "dependencies": { + "@tupaia/utils": "workspace:*", "https-proxy-agent": "^5.0.1", "node-fetch": "^1.7.3", "winston": "^3.17.0" diff --git a/packages/sync/src/utils/getDependencyOrder.ts b/packages/sync/src/utils/getDependencyOrder.ts index c707e1a927..679a4bfe38 100644 --- a/packages/sync/src/utils/getDependencyOrder.ts +++ b/packages/sync/src/utils/getDependencyOrder.ts @@ -1,6 +1,7 @@ import { compact, groupBy, mapValues } from 'lodash'; -import { BaseDatabase } from '@tupaia/database'; +import { BaseDatabase, DatabaseModel } from '@tupaia/database'; +import { isNotNullish } from '@tupaia/tsutils'; interface Dependency { table_name: string; @@ -11,36 +12,29 @@ export async function getDependencyOrder(database: BaseDatabase): Promise => { + const orderedDependencies = await getDependencyOrder(models[0].database); + const recordNames = new Set(models.map(r => r.databaseRecord)); + + return orderedDependencies + .filter(dep => recordNames.has(dep)) + .map(dep => models.find(r => r.databaseRecord === dep)) + .filter(isNotNullish); +}; diff --git a/packages/sync/src/utils/manageSnapshotTable.ts b/packages/sync/src/utils/manageSnapshotTable.ts index 9142099969..554ff44975 100644 --- a/packages/sync/src/utils/manageSnapshotTable.ts +++ b/packages/sync/src/utils/manageSnapshotTable.ts @@ -56,7 +56,7 @@ export const createClientSnapshotTable = async (database: TupaiaDatabase, sessio await database.executeSql(` CREATE TABLE ${tableName} ( id BIGSERIAL PRIMARY KEY, - record_type character varying(255) NOT NULL, + record_type TEXT NOT NULL, is_deleted BOOLEAN DEFAULT false, data json NOT NULL ) WITH ( diff --git a/packages/sync/src/utils/saveChanges.ts b/packages/sync/src/utils/saveChanges.ts index 295a9ecdac..4c2165c2ef 100644 --- a/packages/sync/src/utils/saveChanges.ts +++ b/packages/sync/src/utils/saveChanges.ts @@ -8,8 +8,22 @@ export const saveCreates = async ( ) => { for (let i = 0; i < records.length; i += batchSize) { const batch = records.slice(i, i + batchSize); - await model.createMany(batch); - progressCallback?.(batch.length); + try { + await model.createMany(batch); + progressCallback?.(batch.length); + } catch (e: any) { + // try records individually, some may succeed and we want to capture the + // specific one with the error + await Promise.all( + batch.map(async row => { + try { + await model.create(row); + } catch (error: any) { + throw new Error(`Insert failed with '${error.message}', recordId: ${row.id}`); + } + }), + ); + } } }; @@ -23,8 +37,23 @@ export const saveUpdates = async ( for (let i = 0; i < recordsToSave.length; i += batchSize) { const batch = recordsToSave.slice(i, i + batchSize); - await Promise.all(batch.map(r => model.update({ id: r.id }, r))); - progressCallback?.(batch.length); + try { + await Promise.all(batch.map(r => model.updateById(r.id, r))); + // await Promise.all(batch.map(r => model.updateById(r.id, r))); + progressCallback?.(batch.length); + } catch (e) { + // try records individually, some may succeed and we want to capture the + // specific one with the error + await Promise.all( + batch.map(async row => { + try { + await model.updateById(row.id, row); + } catch (error: any) { + throw new Error(`Update failed with '${error.message}', recordId: ${row.id}`); + } + }), + ); + } } }; @@ -36,7 +65,22 @@ export const saveDeletes = async ( ) => { for (let i = 0; i < recordsForDelete.length; i += batchSize) { const batch = recordsForDelete.slice(i, i + batchSize); - await model.delete({ id: batch.map(r => r.id) }); - progressCallback?.(batch.length); + try { + await model.delete({ id: batch.map(r => r.id) }); + + progressCallback?.(batch.length); + } catch (e) { + // try records individually, some may succeed and we want to capture the + // specific one with the error + await Promise.all( + batch.map(async row => { + try { + await model.delete({ id: row.id }); + } catch (error: any) { + throw new Error(`Delete failed with '${error.message}', recordId: ${row.id}`); + } + }), + ); + } } }; diff --git a/packages/sync/src/utils/saveIncomingChanges.ts b/packages/sync/src/utils/saveIncomingChanges.ts index 7c382c6d46..24f01ebdb6 100644 --- a/packages/sync/src/utils/saveIncomingChanges.ts +++ b/packages/sync/src/utils/saveIncomingChanges.ts @@ -7,6 +7,7 @@ import { sleep } from '@tupaia/utils'; import { saveCreates, saveDeletes, saveUpdates } from './saveChanges'; import { ModelSanitizeArgs, RecordType, SyncSnapshotAttributes } from '../types'; import { findSyncSnapshotRecords } from './findSyncSnapshotRecords'; +import { sortModelsByDependencyOrder } from './getDependencyOrder'; // TODO: Move this to a config model RN-1668 const PERSISTED_CACHE_BATCH_SIZE = 10000; @@ -130,8 +131,9 @@ export const saveIncomingSnapshotChanges = async ( } assertIsWithinTransaction(models[0].database); + const sortedModels = await sortModelsByDependencyOrder(models); - for (const model of models) { + for (const model of sortedModels) { await saveChangesForModelInBatches( model, sessionId, diff --git a/packages/tsutils/src/task/formatFilters.ts b/packages/tsutils/src/task/formatFilters.ts index f0670225ab..394492723f 100644 --- a/packages/tsutils/src/task/formatFilters.ts +++ b/packages/tsutils/src/task/formatFilters.ts @@ -32,7 +32,6 @@ export const formatFilters = (filters: Record[]) => { const end = new Date(value); // subtract 23 hours, 59 minutes, 59 seconds to get the start of the day. This is because the filters always send the end of the day, and we need a range to handle the values being saved in the database as unix timestamps based on the user's timezone. const start = sub(end, { hours: 23, minutes: 59, seconds: 59 }); - formattedFilters[id] = { comparator: 'BETWEEN', comparisonValue: [start.getTime(), end.getTime()], diff --git a/packages/types/src/schemas/schemas.ts b/packages/types/src/schemas/schemas.ts index 810ea71145..5be2da5970 100644 --- a/packages/types/src/schemas/schemas.ts +++ b/packages/types/src/schemas/schemas.ts @@ -41915,6 +41915,9 @@ export const CountryCreateSchema = { }, "name": { "type": "string" + }, + "updated_at_sync_tick": { + "type": "string" } }, "additionalProperties": false, @@ -41934,6 +41937,9 @@ export const CountryUpdateSchema = { }, "name": { "type": "string" + }, + "updated_at_sync_tick": { + "type": "string" } }, "additionalProperties": false diff --git a/packages/types/src/types/models.ts b/packages/types/src/types/models.ts index e3ba45e5fb..4b6fefeb6b 100644 --- a/packages/types/src/types/models.ts +++ b/packages/types/src/types/models.ts @@ -266,11 +266,13 @@ export interface Country { export interface CountryCreate { 'code': string; 'name': string; + 'updated_at_sync_tick'?: string; } export interface CountryUpdate { 'code'?: string; 'id'?: string; 'name'?: string; + 'updated_at_sync_tick'?: string; } export interface Dashboard { 'code': string; diff --git a/yarn.lock b/yarn.lock index c4f91feb74..919d565a12 100644 --- a/yarn.lock +++ b/yarn.lock @@ -13036,6 +13036,7 @@ __metadata: version: 0.0.0-use.local resolution: "@tupaia/superset-api@workspace:packages/superset-api" dependencies: + "@tupaia/utils": "workspace:*" "@types/jest": ^29.5.14 https-proxy-agent: ^5.0.1 jest: ^29.7.0