Skip to content

Commit 5cf2854

Browse files
committed
Address snapshot count review feedback
1 parent c54433c commit 5cf2854

3 files changed

Lines changed: 64 additions & 19 deletions

File tree

src/analysis/individualStudy/management/DataManagementItem.tsx

Lines changed: 41 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,41 @@ type SnapshotAction =
2323
| { type: 'deleteSnapshot', snapshot: string }
2424
| { type: 'deleteLive' };
2525

26+
function getSnapshotStudyId(snapshotKey: string) {
27+
return snapshotKey.slice(snapshotKey.indexOf('-') + 1);
28+
}
29+
30+
function getSnapshotDateKey(snapshotName: string): string | null {
31+
const regex = /-snapshot-(.+)$/;
32+
const match = snapshotName.match(regex);
33+
34+
return match?.[1] ?? null;
35+
}
36+
37+
function getDateFromSnapshotName(snapshotName: string): string | null {
38+
return getSnapshotDateKey(snapshotName)?.replace('T', ' ') ?? null;
39+
}
40+
41+
function compareSnapshotsByDate(
42+
[leftKey]: [string, SnapshotDocContent[string]],
43+
[rightKey]: [string, SnapshotDocContent[string]],
44+
) {
45+
const leftDate = getSnapshotDateKey(leftKey);
46+
const rightDate = getSnapshotDateKey(rightKey);
47+
48+
if (!leftDate && !rightDate) {
49+
return leftKey.localeCompare(rightKey);
50+
}
51+
if (!leftDate) {
52+
return 1;
53+
}
54+
if (!rightDate) {
55+
return -1;
56+
}
57+
58+
return rightDate.localeCompare(leftDate);
59+
}
60+
2661
export function DataManagementItem({ studyId, refresh }: { studyId: string, refresh: () => Promise<ParticipantDataWithStatus[]> }) {
2762
const [modalArchiveOpened, setModalArchiveOpened] = useState<boolean>(false);
2863
const [modalDeleteSnapshotOpened, setModalDeleteSnapshotOpened] = useState<boolean>(false);
@@ -72,9 +107,8 @@ export function DataManagementItem({ studyId, refresh }: { studyId: string, refr
72107

73108
snapshotCountBackfills.current.add(snapshotKey);
74109
setSnapshotCountStatus((previous) => ({ ...previous, [snapshotKey]: 'loading' }));
75-
const strippedFilename = snapshotKey.slice(snapshotKey.indexOf('-') + 1);
76110

77-
storageEngine.getAllParticipantsData(strippedFilename)
111+
storageEngine.getAllParticipantsData(getSnapshotStudyId(snapshotKey))
78112
.then(async (participants) => {
79113
const participantCounts = calculateSnapshotParticipantCounts(participants);
80114
await storageEngine.updateSnapshotParticipantCounts(studyId, snapshotKey, participantCounts);
@@ -104,6 +138,7 @@ export function DataManagementItem({ studyId, refresh }: { studyId: string, refr
104138
})
105139
.catch((error) => {
106140
console.error(`Failed to backfill participant counts for snapshot ${snapshotKey}:`, error);
141+
snapshotCountBackfills.current.delete(snapshotKey);
107142
if (!isCancelled) {
108143
setSnapshotCountStatus((previous) => ({ ...previous, [snapshotKey]: 'failed' }));
109144
}
@@ -188,21 +223,9 @@ export function DataManagementItem({ studyId, refresh }: { studyId: string, refr
188223
onConfirm: () => handleRestoreSnapshot(snapshot),
189224
});
190225

191-
const getDateFromSnapshotName = (snapshotName: string): string | null => {
192-
const regex = /-snapshot-(.+)$/;
193-
const match = snapshotName.match(regex);
194-
195-
if (match && match[1]) {
196-
const dateStuff = match[1];
197-
return dateStuff.replace('T', ' ');
198-
}
199-
return null;
200-
};
201-
202-
const fetchParticipants = async (snapshotName: string) => {
203-
const strippedFilename = snapshotName.slice(snapshotName.indexOf('-') + 1);
204-
return await storageEngine.getAllParticipantsData(strippedFilename);
205-
};
226+
const fetchParticipants = async (snapshotName: string) => (
227+
await storageEngine.getAllParticipantsData(getSnapshotStudyId(snapshotName))
228+
);
206229

207230
const renderParticipantCount = (
208231
snapshotKey: string,
@@ -318,7 +341,7 @@ export function DataManagementItem({ studyId, refresh }: { studyId: string, refr
318341
</Table.Tr>
319342
</Table.Thead>
320343
<Table.Tbody>
321-
{Object.entries(snapshots).map(
344+
{Object.entries(snapshots).sort(compareSnapshotsByDate).map(
322345
([key, snapshotItem]) => (
323346
<Table.Tr key={key}>
324347
<Table.Td>{snapshotItem.name}</Table.Td>

src/analysis/individualStudy/management/tests/ManageView.spec.tsx

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -354,6 +354,28 @@ describe('ManageView', () => {
354354
expect(screen.getByText('DownloadButtons')).toBeDefined();
355355
});
356356

357+
test('DataManagementItem sorts snapshots by newest creation date first', async () => {
358+
mockStorageEngine!.getSnapshots.mockResolvedValue({
359+
'test-study-snapshot-2026-06-09T01:00:00': {
360+
name: 'older-snapshot',
361+
participantCounts: { completed: 1, inProgress: 0, rejected: 0 },
362+
},
363+
'test-study-snapshot-2026-06-10T01:00:00': {
364+
name: 'newer-snapshot',
365+
participantCounts: { completed: 2, inProgress: 0, rejected: 0 },
366+
},
367+
});
368+
369+
await act(async () => {
370+
render(<DataManagementItem studyId="test-study" refresh={async () => []} />);
371+
});
372+
373+
expect(screen.getAllByText(/-snapshot$/).map((element) => element.textContent)).toEqual([
374+
'newer-snapshot',
375+
'older-snapshot',
376+
]);
377+
});
378+
357379
test('DataManagementItem backfills missing snapshot participant counts', async () => {
358380
mockStorageEngine!.getSnapshots.mockResolvedValue({
359381
'dev-test-study-snapshot-2026T01:00': { name: 'my-snapshot' },

src/storage/engines/types.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -972,7 +972,7 @@ export abstract class StorageEngine {
972972

973973
// Gets all participant IDs for the given studyId
974974
async getAllParticipantIds(studyId?: string) {
975-
const studyIdToUse = this.studyId || studyId;
975+
const studyIdToUse = studyId ?? this.studyId;
976976
if (studyIdToUse === undefined) {
977977
throw new Error('Study ID is not set');
978978
}

0 commit comments

Comments
 (0)