Skip to content

fix(admin): address review feedback on admin dashboard stats #359

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
mustafaneguib merged 2 commits into from
Mar 6, 2026
Merged

fix(admin): address review feedback on admin dashboard stats #359

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5926b62
Initial plan
Copilot Mar 6, 2026
19eb108
fix: address all PR review comments on admin dashboard
Copilot Mar 6, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
92 changes: 47 additions & 45 deletions backend/src/processors/AdminStatsProcessor.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { AppDataSource } from '../datasources/PostgresDS.js';
import { DRADataSource } from '../models/DRADataSource.js';
import { getRedisClient } from '../config/redis.config.js';
import { ScheduledBackupProcessor } from './ScheduledBackupProcessor.js';
import { ScheduledBackupService } from '../services/ScheduledBackupService.js';
Expand Down Expand Up @@ -151,8 +152,9 @@ export class AdminStatsProcessor {
let activeRedisSessions = 0;
try {
const redis = getRedisClient();
const keys = await redis.keys('dra:ai:*session*');
activeRedisSessions = keys.length;
for await (const _key of redis.scanIterator({ MATCH: 'dra:ai:*session*' })) {
activeRedisSessions++;
}
} catch {
activeRedisSessions = 0;
}
Expand Down Expand Up @@ -183,63 +185,63 @@ export class AdminStatsProcessor {
};
}

private async querySyncHealthSummary(manager: any) {
const rows = await manager.query(`
SELECT
COUNT(*)::int AS total,
COUNT(*) FILTER (
WHERE data_type NOT IN ('postgresql','mysql','mariadb','mongodb','csv','excel','pdf')
AND (
connection_details->'api_connection_details'->'api_config'->>'last_sync' IS NULL
OR connection_details->'api_connection_details'->'api_config'->>'last_sync' = 'null'
)
)::int AS never_synced,
0::int AS failed
FROM dra_data_sources
`);
const r = rows[0] || {};
private async querySyncHealthSummary(_manager: any) {
const FILE_DB_TYPES = ['postgresql', 'mysql', 'mariadb', 'mongodb', 'csv', 'excel', 'pdf'];
const dataSources = await AppDataSource.manager.find(DRADataSource);
Comment on lines +188 to +190
Copy link

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

querySyncHealthSummary(_manager) now ignores the passed manager and uses AppDataSource.manager directly. This makes it harder to reuse inside transactions and reduces testability/mocking. Prefer using the injected manager (i.e., manager.find(DRADataSource, ...)) or remove the unused parameter to avoid confusion.

Suggested change
private async querySyncHealthSummary(_manager: any) {
const FILE_DB_TYPES = ['postgresql', 'mysql', 'mariadb', 'mongodb', 'csv', 'excel', 'pdf'];
const dataSources = await AppDataSource.manager.find(DRADataSource);
private async querySyncHealthSummary(manager: any) {
const FILE_DB_TYPES = ['postgresql', 'mysql', 'mariadb', 'mongodb', 'csv', 'excel', 'pdf'];
const dataSources = await manager.find(DRADataSource);

Copilot uses AI. Check for mistakes.

const total = dataSources.length;
let neverSynced = 0;
let failedSources = 0;

for (const ds of dataSources) {
if (FILE_DB_TYPES.includes(ds.data_type)) continue;
Comment on lines +190 to +197
Copy link

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

querySyncHealthSummary() loads and decrypts all DRADataSource rows into memory (find(DRADataSource)), which can become expensive as the number of data sources grows (especially since connection_details is encrypted/decrypted). Consider limiting the selected columns to only what’s needed (data_type, connection_details) and/or scoping the query to only API-integrated sources (excluding FILE_DB_TYPES) to reduce I/O and decryption work.

Suggested change
const dataSources = await AppDataSource.manager.find(DRADataSource);
const total = dataSources.length;
let neverSynced = 0;
let failedSources = 0;
for (const ds of dataSources) {
if (FILE_DB_TYPES.includes(ds.data_type)) continue;
// Count all data sources cheaply without decrypting connection_details
const total = await AppDataSource.manager.count(DRADataSource);
// Only load the minimal fields needed for sync health, and only for API-integrated sources
const apiDataSources = await AppDataSource.manager
.createQueryBuilder(DRADataSource, 'ds')
.select(['ds.data_type', 'ds.connection_details'])
.where('ds.data_type NOT IN (:...fileTypes)', { fileTypes: FILE_DB_TYPES })
.getMany();
let neverSynced = 0;
let failedSources = 0;
for (const ds of apiDataSources) {

Copilot uses AI. Check for mistakes.
const lastSync = ds.connection_details?.api_connection_details?.api_config?.last_sync;
if (!lastSync) {
neverSynced++;
} else {
const hoursSinceSync = (Date.now() - new Date(lastSync as any).getTime()) / 3600000;
if (hoursSinceSync > 72) failedSources++;
Comment on lines +198 to +203
Copy link

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

querySyncHealthSummary() treats any truthy last_sync value as a valid date. If legacy rows contain the string 'null' (which the previous SQL explicitly checked for), new Date('null') becomes an invalid date and the source won’t be counted as neverSynced or failedSources. Handle 'null' (and other invalid dates) the same way as missing last_sync.

Suggested change
const lastSync = ds.connection_details?.api_connection_details?.api_config?.last_sync;
if (!lastSync) {
neverSynced++;
} else {
const hoursSinceSync = (Date.now() - new Date(lastSync as any).getTime()) / 3600000;
if (hoursSinceSync > 72) failedSources++;
const lastSyncRaw = ds.connection_details?.api_connection_details?.api_config?.last_sync;
const lastSync = lastSyncRaw ? String(lastSyncRaw) : null;
if (!lastSync || lastSync === 'null') {
// Treat missing or legacy 'null' values as never synced
neverSynced++;
} else {
const parsedDate = new Date(lastSync);
const timestamp = parsedDate.getTime();
if (Number.isNaN(timestamp)) {
// Invalid dates are also treated as never synced
neverSynced++;
} else {
const hoursSinceSync = (Date.now() - timestamp) / 3600000;
if (hoursSinceSync > 72) failedSources++;
}

Copilot uses AI. Check for mistakes.
}
}

return {
totalSources: r.total || 0,
failedSources: r.failed || 0,
neverSynced: r.never_synced || 0,
totalSources: total,
failedSources,
neverSynced,
};
}

async getSyncHealthData(): Promise<IDataSourceSyncRow[]> {
const manager = AppDataSource.manager;
const rows = await manager.query(`
SELECT
ds.id,
ds.name,
ds.data_type,
ds.created_at,
u.email AS owner_email,
ds.connection_details->'api_connection_details'->'api_config'->>'last_sync' AS last_sync
FROM dra_data_sources ds
LEFT JOIN dra_users_platform u ON ds.users_platform_id = u.id
ORDER BY ds.id DESC
LIMIT 200
`);
const FILE_DB_TYPES = ['postgresql', 'mysql', 'mariadb', 'mongodb', 'csv', 'excel', 'pdf'];

const dataSources = await AppDataSource.manager.find(DRADataSource, {
relations: ['users_platform'],
order: { id: 'DESC' },
take: 200,
});

return dataSources.map((ds) => {
const isFileOrDb = FILE_DB_TYPES.includes(ds.data_type);
const lastSyncRaw = ds.connection_details?.api_connection_details?.api_config?.last_sync;
const lastSync = lastSyncRaw ? String(lastSyncRaw) : null;

return rows.map((r: any) => {
const isFileOrDb = ['postgresql', 'mysql', 'mariadb', 'mongodb', 'csv', 'excel', 'pdf'].includes(r.data_type);
let status: 'synced' | 'failed' | 'never' = 'synced';
if (!isFileOrDb) {
if (!r.last_sync || r.last_sync === 'null') {
if (!lastSync || lastSync === 'null') {
status = 'never';
} else {
const lastSyncDate = new Date(r.last_sync);
const hoursSinceSync = (Date.now() - lastSyncDate.getTime()) / 3600000;
const hoursSinceSync = (Date.now() - new Date(lastSync).getTime()) / 3600000;
status = hoursSinceSync > 72 ? 'failed' : 'synced';
}
}

return {
id: r.id,
name: r.name,
data_type: r.data_type,
owner_email: r.owner_email || 'Unknown',
last_sync: r.last_sync || null,
created_at: r.created_at || null,
id: ds.id,
name: ds.name,
data_type: ds.data_type,
owner_email: (ds as any).users_platform?.email || 'Unknown',
Copy link

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

owner_email is pulled via (ds as any).users_platform?.email even though DRADataSource already declares a typed users_platform relation and this query is loading it via relations. Dropping the any cast will keep this type-safe and make it easier to spot relation-loading issues at compile time.

Suggested change
owner_email: (ds as any).users_platform?.email || 'Unknown',
owner_email: ds.users_platform?.email || 'Unknown',

Copilot uses AI. Check for mistakes.
last_sync: lastSync,
created_at: ds.created_at ? String(ds.created_at) : null,
status,
Comment on lines +223 to 245
Copy link

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

getSyncHealthData() serializes timestamps using String(...) (e.g. created_at: String(ds.created_at), last_sync: String(lastSyncRaw)). String(Date) yields a locale/implementation-dependent format; returning a stable ISO-8601 string (e.g. toISOString()) is safer for clients and consistent with other parts of the backend.

Copilot uses AI. Check for mistakes.
};
});
Expand Down
8 changes: 4 additions & 4 deletions frontend/components/AdminChart.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,10 @@ async function renderChart() {
.attr('width', w)
.attr('height', h);

cleanup = () => {
svg.remove();
};

const g = svg.append('g').attr('transform', `translate(${margin.left},${margin.top})`);

if (props.type === 'donut') {
Expand Down Expand Up @@ -138,10 +142,6 @@ async function renderChart() {
.attr('r', 3)
.attr('fill', color);
}

cleanup = () => {
svg.remove();
};
}

onMounted(() => {
Expand Down
63 changes: 6 additions & 57 deletions frontend/composables/useAdminStats.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,11 @@ import type {
IAdminOverviewStats,
IDataSourceSyncRow,
ISystemHealthStatus,
ITimeSeriesPoint,
} from '~/types/admin/stats';

export const useAdminStats = () => {
const config = useRuntimeConfig();
const { $socket } = useNuxtApp() as any;
const { $socketio } = useNuxtApp() as any;

const overviewStats = ref<IAdminOverviewStats | null>(null);
const syncHealthData = ref<IDataSourceSyncRow[]>([]);
Expand Down Expand Up @@ -75,19 +74,19 @@ export const useAdminStats = () => {
if (!import.meta.client) return;
loadAll();

if ($socket) {
$socket.emit('join-admin-room');
if ($socketio) {
$socketio.emit('join-admin-room');
statsUpdateHandler = () => {
refreshStats();
};
$socket.on('admin-stats-update', statsUpdateHandler);
$socketio.on('admin-stats-update', statsUpdateHandler);
}
});

onUnmounted(() => {
if (!import.meta.client) return;
if ($socket && statsUpdateHandler) {
$socket.off('admin-stats-update', statsUpdateHandler);
if ($socketio && statsUpdateHandler) {
$socketio.off('admin-stats-update', statsUpdateHandler);
}
});

Expand All @@ -100,53 +99,3 @@ export const useAdminStats = () => {
refreshStats,
};
};

export const useAdminTimeSeries = (metric: string, days = 30) => {
const config = useRuntimeConfig();
const data = ref<ITimeSeriesPoint[]>([]);
const dsTypeBreakdown = ref<{ data_type: string; count: number }[]>([]);
const isLoading = ref(false);

const authHeaders = (): Record<string, string> => {
const token = getAuthToken();
if (!token) throw new Error('Authentication required');
return {
Authorization: `Bearer ${token}`,
'Authorization-Type': 'auth',
};
};

const fetchTimeSeries = async () => {
const res = await $fetch<{ success: boolean; data: ITimeSeriesPoint[] }>(
`${config.public.apiBase}/admin/stats/timeseries?metric=${metric}&days=${days}`,
{ headers: authHeaders() }
);
if (res.success) data.value = res.data;
};

const fetchDsTypeBreakdown = async () => {
const res = await $fetch<{ success: boolean; data: { data_type: string; count: number }[] }>(
`${config.public.apiBase}/admin/stats/datasource-types`,
{ headers: authHeaders() }
);
if (res.success) dsTypeBreakdown.value = res.data;
};

onMounted(async () => {
if (!import.meta.client) return;
isLoading.value = true;
try {
if (metric === 'datasource_types') {
await fetchDsTypeBreakdown();
} else {
await fetchTimeSeries();
}
} catch (err) {
console.error('[useAdminTimeSeries] Load failed:', err);
} finally {
isLoading.value = false;
}
});

return { data, dsTypeBreakdown, isLoading };
};
44 changes: 35 additions & 9 deletions frontend/pages/admin/index.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,22 +34,48 @@ const authHeaders = () => {
const fetchAllSeries = async () => {
isSeriesLoading.value = true;
try {
const [signups, projects, ai, cancellations, dsTypes] = await Promise.all([
const results = await Promise.allSettled([
$fetch<any>(`${config.public.apiBase}/admin/stats/timeseries?metric=signups&days=30`, { headers: authHeaders() }),
$fetch<any>(`${config.public.apiBase}/admin/stats/timeseries?metric=projects&days=30`, { headers: authHeaders() }),
$fetch<any>(`${config.public.apiBase}/admin/stats/timeseries?metric=ai_messages&days=30`, { headers: authHeaders() }),
$fetch<any>(`${config.public.apiBase}/admin/stats/timeseries?metric=cancellations&days=30`, { headers: authHeaders() }),
$fetch<any>(`${config.public.apiBase}/admin/stats/datasource-types`, { headers: authHeaders() }),
Comment on lines 34 to 42
Copy link

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fetchAllSeries() no longer has a catch block. While Promise.allSettled() won’t reject, synchronous failures (e.g. authHeaders() throwing when the token is missing/expired) will now propagate and can break the page instead of being logged/handled. Consider adding a catch to set an error state/log once for unexpected exceptions.

Copilot uses AI. Check for mistakes.
]);
if (signups.success) signupSeries.value = signups.data;
if (projects.success) projectSeries.value = projects.data;
if (ai.success) aiSeries.value = ai.data;
if (cancellations.success) cancellationSeries.value = cancellations.data;
if (dsTypes.success) {
dsTypeSeries.value = dsTypes.data.map((d: any) => ({ label: d.data_type, value: d.count }));

const [signupsResult, projectsResult, aiResult, cancellationsResult, dsTypesResult] = results;

if (signupsResult.status === 'fulfilled' && signupsResult.value.success) {
signupSeries.value = signupsResult.value.data;
} else if (signupsResult.status === 'rejected') {
console.error('[AdminDashboard] Failed to load signups time-series:', signupsResult.reason);
}

if (projectsResult.status === 'fulfilled' && projectsResult.value.success) {
projectSeries.value = projectsResult.value.data;
} else if (projectsResult.status === 'rejected') {
console.error('[AdminDashboard] Failed to load projects time-series:', projectsResult.reason);
}

if (aiResult.status === 'fulfilled' && aiResult.value.success) {
aiSeries.value = aiResult.value.data;
} else if (aiResult.status === 'rejected') {
console.error('[AdminDashboard] Failed to load AI messages time-series:', aiResult.reason);
}

if (cancellationsResult.status === 'fulfilled' && cancellationsResult.value.success) {
cancellationSeries.value = cancellationsResult.value.data;
} else if (cancellationsResult.status === 'rejected') {
console.error('[AdminDashboard] Failed to load cancellations time-series:', cancellationsResult.reason);
}

if (dsTypesResult.status === 'fulfilled' && dsTypesResult.value.success) {
dsTypeSeries.value = dsTypesResult.value.data.map((d: any) => ({
label: d.data_type,
value: d.count,
}));
} else if (dsTypesResult.status === 'rejected') {
console.error('[AdminDashboard] Failed to load data source type stats:', dsTypesResult.reason);
}
} catch (err) {
console.error('[AdminDashboard] Failed to load time-series:', err);
} finally {
isSeriesLoading.value = false;
}
Expand Down