fix(admin): address review feedback on admin dashboard stats by Copilot · Pull Request #359 · Data-Research-Analysis/data-research-analysis-platform

Copilot · 2026-03-06T20:43:17Z

Seven issues identified in review of the admin dashboard PR — broken Socket.IO subscription, encryption bypass via raw SQL, dead code, and resilience gaps.

Backend (`AdminStatsProcessor`)

Encryption fix (critical): querySyncHealthSummary and getSyncHealthData were using raw SQL JSON path operators (connection_details->'api_connection_details'->...) on an AES-encrypted column — always returning NULL in production. Replaced with TypeORM manager.find(DRADataSource) to get automatic decryption via the ValueTransformer.
failedSources always 0: Replaced hardcoded 0::int AS failed with application-level stale detection using the same 72-hour threshold already used in getSyncHealthData.
Blocking Redis scan: Replaced redis.keys('dra:ai:*session*') (O(N), blocks Redis) with redis.scanIterator({ MATCH: 'dra:ai:*session*' }).

Frontend

Broken Socket.IO subscription (useAdminStats): Plugin provides $socketio (not $socket) — the join-admin-room emit and admin-stats-update listener were never registered. Fixed the destructuring reference.
Dead code: Removed the exported useAdminTimeSeries composable — the dashboard page fetches time-series data inline and this was never called.
Partial failure resilience (admin/index.vue): Replaced Promise.all with Promise.allSettled for the 5 time-series fetches so one failing endpoint doesn't blank all charts.
Donut chart cleanup gap (AdminChart.vue): Moved cleanup = () => svg.remove() to immediately after SVG creation (before the donut early return) so the cleanup callback is always set for all chart types.

💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

mustafaneguib · 2026-03-06T20:46:24Z

@copilot what is the status of this pull request?

- Fix $socket -> $socketio in useAdminStats.ts (bug: live updates never worked) - Remove unused useAdminTimeSeries composable from useAdminStats.ts - Replace Promise.all with Promise.allSettled in admin/index.vue fetchAllSeries - Move cleanup assignment before donut chart early-return in AdminChart.vue - Fix encryption issue: replace raw SQL JSON operators with TypeORM find() in querySyncHealthSummary and getSyncHealthData (connection_details is encrypted) - Fix hardcoded failedSources=0: compute stale count with 72h threshold - Use redis.scanIterator instead of blocking redis.keys for AI session count Co-authored-by: mustafaneguib <1302045+mustafaneguib@users.noreply.github.com>

Copilot · 2026-03-06T20:51:59Z

@copilot what is the status of this pull request?

PR #359 is open (draft), targeting DRA-272-Build-Admin-Dashboard. All 7 review items have been addressed in commit 19eb108:

#	Issue	Status
1	`$socket` → `$socketio` (live stats subscription broken)	✅ Fixed
2	Remove unused `useAdminTimeSeries` composable	✅ Fixed
3	`Promise.all` → `Promise.allSettled` for time-series fetches	✅ Fixed
4	Donut chart cleanup missing before early `return`	✅ Fixed
5	Encryption bypass via raw SQL in `querySyncHealthSummary`	✅ Fixed
6	Encryption bypass via raw SQL in `getSyncHealthData`	✅ Fixed
7	Hardcoded `failedSources = 0`	✅ Fixed
8	Blocking `redis.keys` → `redis.scanIterator`	✅ Fixed

The CI workflow run for 19eb108 is currently in progress. CodeQL scan on the changes returned 0 alerts.

Copilot

Pull request overview

This PR addresses review feedback on the admin dashboard stats by fixing an encryption-related backend bug (raw SQL against encrypted connection_details), restoring correct Socket.IO subscriptions in the frontend, improving resilience of chart data loading, and removing dead code/ensuring chart cleanup runs consistently.

Changes:

Backend: replace raw SQL JSON-path queries on encrypted connection_details with TypeORM reads and add stale-sync detection.
Frontend: fix Socket.IO usage ($socketio), make admin time-series loading resilient via Promise.allSettled, and remove unused useAdminTimeSeries.
Chart rendering: ensure D3 SVG cleanup is always registered (including donut early-return path).

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 6 comments.

File	Description
frontend/pages/admin/index.vue	Switches time-series loading to `Promise.allSettled` to avoid one failing endpoint blanking all charts.
frontend/composables/useAdminStats.ts	Fixes Socket.IO injection reference (`$socketio`) and removes unused time-series composable export.
frontend/components/AdminChart.vue	Ensures cleanup callback is set immediately after SVG creation so donut charts don’t leak DOM.
backend/src/processors/AdminStatsProcessor.ts	Replaces raw SQL JSON operators on encrypted data with TypeORM reads; improves Redis session counting and sync health logic.

Copilot · 2026-03-06T21:03:03Z

+        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',
+                last_sync: lastSync,
+                created_at: ds.created_at ? String(ds.created_at) : null,
                status,


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 · 2026-03-06T21:03:04Z

+                id: ds.id,
+                name: ds.name,
+                data_type: ds.data_type,
+                owner_email: (ds as any).users_platform?.email || 'Unknown',


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 · 2026-03-06T21:03:04Z

 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() }),


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 · 2026-03-06T21:03:04Z

+            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++;


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 · 2026-03-06T21:03:05Z

+        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;


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 · 2026-03-06T21:03:05Z

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


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);

Initial plan

5926b62

Copilot AI assigned Copilot and mustafaneguib Mar 6, 2026

Copilot AI mentioned this pull request Mar 6, 2026

DRA-272-Build-Admin-Dashboard #358

Merged

15 tasks

Copilot started work on behalf of mustafaneguib March 6, 2026 20:43 View session

Copilot AI changed the title ~~[WIP] Implement admin dashboard with real-time platform statistics~~ fix(admin): address review feedback on admin dashboard stats Mar 6, 2026

Copilot finished work on behalf of mustafaneguib March 6, 2026 20:50

Copilot started work on behalf of mustafaneguib March 6, 2026 20:51 View session

Copilot finished work on behalf of mustafaneguib March 6, 2026 20:52

mustafaneguib marked this pull request as ready for review March 6, 2026 20:59

Copilot AI review requested due to automatic review settings March 6, 2026 20:59

mustafaneguib merged commit d470e3a into DRA-272-Build-Admin-Dashboard Mar 6, 2026
2 checks passed

mustafaneguib deleted the copilot/sub-pr-358 branch March 6, 2026 20:59

Copilot started reviewing on behalf of mustafaneguib March 6, 2026 21:00 View session

Copilot AI reviewed Mar 6, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

fix(admin): address review feedback on admin dashboard stats#359
mustafaneguib merged 2 commits intoDRA-272-Build-Admin-Dashboardfrom
copilot/sub-pr-358

Copilot AI commented Mar 6, 2026 •

edited

Loading

Uh oh!

mustafaneguib commented Mar 6, 2026

Uh oh!

Copilot AI commented Mar 6, 2026

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 6, 2026

Uh oh!

Copilot AI Mar 6, 2026

Uh oh!

Copilot AI Mar 6, 2026

Uh oh!

Copilot AI Mar 6, 2026

Uh oh!

Copilot AI Mar 6, 2026

Uh oh!

Copilot AI Mar 6, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

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

-        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) {

Conversation

Copilot AI commented Mar 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Backend (AdminStatsProcessor)

Frontend

Uh oh!

mustafaneguib commented Mar 6, 2026

Uh oh!

Copilot AI commented Mar 6, 2026

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Copilot AI commented Mar 6, 2026 •

edited

Loading

Backend (`AdminStatsProcessor`)