refactor: code review changes for upload instrument feature

Author: joshunrau

apps/api/src/instrument-records/dto/upload-instrument-record.dto.ts

@@ -1,9 +1,9 @@
 import { ValidationSchema } from '@douglasneuroinformatics/libnest/core';
 import type { Json } from '@opendatacapture/schemas/core';
-import { $UploadInstrumentRecordData } from '@opendatacapture/schemas/instrument-records';
+import { $UploadInstrumentRecordsData } from '@opendatacapture/schemas/instrument-records';
 
-@ValidationSchema($UploadInstrumentRecordData)
-export class UploadInstrumentRecordDto {
+@ValidationSchema($UploadInstrumentRecordsData)
+export class UploadInstrumentRecordsDto {
   groupId?: string;
   instrumentId: string;
   records: {

apps/api/src/instrument-records/instrument-records.controller.ts

@@ -10,10 +10,9 @@ import { RouteAccess } from '@/core/decorators/route-access.decorator';
 import type { AppAbility } from '@/core/types';
 
 import { CreateInstrumentRecordDto } from './dto/create-instrument-record.dto';
+import { UploadInstrumentRecordsDto } from './dto/upload-instrument-record.dto';
 import { InstrumentRecordsService } from './instrument-records.service';
 
-import { UploadInstrumentRecordDto } from './dto/upload-instrument-record.dto';
-
 @ApiTags('Instrument Records')
 @Controller('instrument-records')
 export class InstrumentRecordsController {
@@ -26,10 +25,10 @@ export class InstrumentRecordsController {
     return this.instrumentRecordsService.create(data, { ability });
   }
 
-  @ApiOperation({ summary: 'Upload multiple instrument records' })
+  @ApiOperation({ summary: 'Upload Multiple Instrument Records' })
   @Post('upload')
   @RouteAccess({ action: 'create', subject: 'InstrumentRecord' })
-  upload(@Body() data: UploadInstrumentRecordDto, @CurrentUser('ability') ability: AppAbility) {
+  upload(@Body() data: UploadInstrumentRecordsDto, @CurrentUser('ability') ability: AppAbility) {
     return this.instrumentRecordsService.upload(data, { ability });
   }
 

apps/api/src/instrument-records/instrument-records.service.ts

@@ -8,11 +8,9 @@ import type {
   InstrumentRecordQueryParams,
   InstrumentRecordsExport,
   LinearRegressionResults,
-  UploadInstrumentRecordData
+  UploadInstrumentRecordsData
 } from '@opendatacapture/schemas/instrument-records';
-import type { SessionType } from '@opendatacapture/schemas/session';
-import type { CreateSubjectData } from '@opendatacapture/schemas/subject';
-import type { InstrumentRecordModel, Prisma } from '@prisma/generated-client';
+import type { InstrumentRecordModel, Prisma, SessionModel } from '@prisma/generated-client';
 import { isNumber, pickBy } from 'lodash-es';
 
 import { accessibleQuery } from '@/ability/ability.utils';
@@ -253,91 +251,106 @@ export class InstrumentRecordsService {
   }
 
   async upload(
-    { groupId, instrumentId, records }: UploadInstrumentRecordData,
+    { groupId, instrumentId, records }: UploadInstrumentRecordsData,
     options?: EntityOperationOptions
   ): Promise<InstrumentRecordModel[]> {
     if (groupId) {
       await this.groupsService.findById(groupId, options);
     }
 
     const instrument = await this.instrumentsService.findById(instrumentId);
-
-    const createdModelsArray: InstrumentRecordModel[] = [];
     if (instrument.kind === 'SERIES') {
       throw new UnprocessableEntityException(
         `Cannot create instrument record for series instrument '${instrument.id}'`
       );
     }
 
-    for (const record of records) {
-      const { data, date, subjectId } = record;
-
-      //create a new subject id in subjects service if they dont exist
-      try {
-        await this.subjectsService.findById(subjectId);
-      } catch (exception) {
-        if (exception instanceof NotFoundException) {
-          const addedSubject: CreateSubjectDto = {
-            id: subjectId
-          };
-          await this.subjectsService.create(addedSubject);
-        } else {
-          throw exception;
-        }
-      }
-
-      let subjectInfo: CreateSubjectData = {
-        dateOfBirth: null,
-        firstName: null,
-        id: subjectId,
-        lastName: null,
-        sex: null
-      };
+    const createdRecordsArray: InstrumentRecordModel[] = [];
+    const createdSessionsArray: SessionModel[] = [];
 
-      let sessionType: SessionType = 'RETROSPECTIVE';
+    try {
+      for (let i = 0; i < records.length; i++) {
+        const { data, date, subjectId } = records[i]!;
+        await this.createSubjectIfNotFound(subjectId);
 
-      let sessionId = (
-        await this.sessionsService.create({
+        const session = await this.sessionsService.create({
           date: date,
           groupId: groupId ? groupId : null,
-          subjectData: subjectInfo,
-          type: sessionType
-        })
-      ).id;
-
-      const createdModel = await this.instrumentRecordModel.create({
-        data: {
-          computedMeasures: instrument.measures
-            ? this.instrumentMeasuresService.computeMeasures(instrument.measures, data)
-            : null,
-          data,
-          date,
-          group: groupId
-            ? {
-                connect: { id: groupId }
-              }
-            : undefined,
-          instrument: {
-            connect: {
-              id: instrumentId
-            }
-          },
-          session: {
-            connect: {
-              id: sessionId
-            }
+          subjectData: {
+            id: subjectId
           },
-          subject: {
-            connect: {
-              id: subjectId
+          type: 'RETROSPECTIVE'
+        });
+
+        createdSessionsArray.push(session);
+
+        const sessionId = session.id;
+
+        if (!instrument.validationSchema.safeParse(data).success) {
+          throw new UnprocessableEntityException(
+            `Data received for record at index '${i}' does not pass validation schema of instrument '${instrument.id}'`
+          );
+        }
+
+        const createdRecord = await this.instrumentRecordModel.create({
+          data: {
+            computedMeasures: instrument.measures
+              ? this.instrumentMeasuresService.computeMeasures(instrument.measures, data)
+              : null,
+            data,
+            date,
+            group: groupId
+              ? {
+                  connect: { id: groupId }
+                }
+              : undefined,
+            instrument: {
+              connect: {
+                id: instrumentId
+              }
+            },
+            session: {
+              connect: {
+                id: sessionId
+              }
+            },
+            subject: {
+              connect: {
+                id: subjectId
+              }
             }
           }
+        });
+
+        createdRecordsArray.push(createdRecord);
+      }
+    } catch (err) {
+      await this.instrumentRecordModel.deleteMany({
+        where: {
+          id: {
+            in: createdRecordsArray.map((record) => record.id)
+          }
         }
       });
-
-      createdModelsArray.push(createdModel);
+      await this.sessionsService.deleteByIds(createdSessionsArray.map((session) => session.id));
+      throw err;
     }
 
-    return createdModelsArray;
+    return createdRecordsArray;
+  }
+
+  private async createSubjectIfNotFound(subjectId: string) {
+    try {
+      await this.subjectsService.findById(subjectId);
+    } catch (exception) {
+      if (exception instanceof NotFoundException) {
+        const addedSubject: CreateSubjectDto = {
+          id: subjectId
+        };
+        await this.subjectsService.create(addedSubject);
+      } else {
+        throw exception;
+      }
+    }
   }
 }

apps/api/src/sessions/sessions.service.ts

@@ -56,6 +56,23 @@ export class SessionsService {
     }))!;
   }
 
+  async deleteById(id: string, { ability }: EntityOperationOptions = {}) {
+    return this.sessionModel.delete({
+      where: { AND: [accessibleQuery(ability, 'delete', 'Session')], id }
+    });
+  }
+
+  async deleteByIds(ids: string[], { ability }: EntityOperationOptions = {}) {
+    return this.sessionModel.deleteMany({
+      where: {
+        AND: [accessibleQuery(ability, 'delete', 'Session')],
+        id: {
+          in: ids
+        }
+      }
+    });
+  }
+
   async findById(id: string, { ability }: EntityOperationOptions = {}) {
     const session = await this.sessionModel.findFirst({
       where: { AND: [accessibleQuery(ability, 'read', 'Session')], id }

apps/web/src/components/Sidebar/Sidebar.tsx

@@ -13,7 +13,6 @@ import { useAppStore } from '@/store';
 import { NavButton } from '../NavButton';
 import { UserDropup } from '../UserDropup';
 
-// eslint-disable-next-line max-lines-per-function
 export const Sidebar = () => {
   const navItems = useNavItems();
   const currentSession = useAppStore((store) => store.currentSession);

apps/web/src/features/about/components/TimeValue.tsx

@@ -9,6 +9,7 @@ export type TimeValueProps = {
 
 export const TimeValue = ({ value }: TimeValueProps) => {
   const format = useCallback((uptime: number) => {
+    // eslint-disable-next-line prefer-const
     let { days, hours, minutes, seconds } = parseDuration(uptime * 1000);
     hours += days * 24;
     return [hours, minutes, seconds].map((value) => (value < 10 ? '0' + value : value)).join(':');

apps/web/src/features/admin/pages/ManageGroupsPage.tsx

@@ -1,11 +1,12 @@
-import { useEffect, useState } from 'react';
+import { useState } from 'react';
 
 import { Button, ClientTable, Heading, SearchBar, Sheet } from '@douglasneuroinformatics/libui/components';
 import { useTranslation } from '@douglasneuroinformatics/libui/hooks';
 import type { Group } from '@opendatacapture/schemas/group';
 import { Link } from 'react-router-dom';
 
 import { PageHeader } from '@/components/PageHeader';
+import { useSearch } from '@/hooks/useSearch';
 
 import { useDeleteGroupMutation } from '../hooks/useDeleteGroupMutation';
 import { useGroupsQuery } from '../hooks/useGroupsQuery';
@@ -15,12 +16,7 @@ export const ManageGroupsPage = () => {
   const groupsQuery = useGroupsQuery();
   const deleteGroupMutation = useDeleteGroupMutation();
   const [selectedGroup, setSelectedGroup] = useState<Group | null>(null);
-  const [groups, setGroups] = useState<Group[]>(groupsQuery.data ?? []);
-  const [searchTerm, setSearchTerm] = useState('');
-
-  useEffect(() => {
-    setGroups((groupsQuery.data ?? []).filter((group) => group.name.toLowerCase().includes(searchTerm.toLowerCase())));
-  }, [groupsQuery.data, searchTerm]);
+  const { filteredData, searchTerm, setSearchTerm } = useSearch(groupsQuery.data ?? [], 'name');
 
   return (
     <Sheet open={Boolean(selectedGroup)} onOpenChange={() => setSelectedGroup(null)}>
@@ -69,7 +65,7 @@ export const ManageGroupsPage = () => {
             label: t('common.groupType')
           }
         ]}
-        data={groups}
+        data={filteredData}
         entriesPerPage={15}
         minRows={15}
         onEntryClick={setSelectedGroup}

apps/web/src/features/admin/pages/ManageUsersPage.tsx

@@ -1,4 +1,4 @@
-import { useEffect, useState } from 'react';
+import { useState } from 'react';
 
 import { snakeToCamelCase } from '@douglasneuroinformatics/libjs';
 import { Button, ClientTable, Heading, SearchBar, Sheet } from '@douglasneuroinformatics/libui/components';
@@ -7,24 +7,19 @@ import type { User } from '@opendatacapture/schemas/user';
 import { Link } from 'react-router-dom';
 
 import { PageHeader } from '@/components/PageHeader';
+import { useSearch } from '@/hooks/useSearch';
 import { useAppStore } from '@/store';
 
 import { useDeleteUserMutation } from '../hooks/useDeleteUserMutation';
 import { useUsersQuery } from '../hooks/useUsersQuery';
 
-// eslint-disable-next-line max-lines-per-function
 export const ManageUsersPage = () => {
   const currentUser = useAppStore((store) => store.currentUser);
   const { t } = useTranslation();
   const usersQuery = useUsersQuery();
   const deleteUserMutation = useDeleteUserMutation();
-  const [users, setUsers] = useState<User[]>(usersQuery.data ?? []);
   const [selectedUser, setSelectedUser] = useState<null | User>(null);
-  const [searchTerm, setSearchTerm] = useState('');
-
-  useEffect(() => {
-    setUsers((usersQuery.data ?? []).filter((user) => user.username.toLowerCase().includes(searchTerm.toLowerCase())));
-  }, [usersQuery.data, searchTerm]);
+  const { filteredData, searchTerm, setSearchTerm } = useSearch(usersQuery.data ?? [], 'username');
 
   const currentUserIsSelected = selectedUser?.username === currentUser?.username;
 
@@ -76,7 +71,7 @@ export const ManageUsersPage = () => {
             label: t('common.basePermissionLevel')
           }
         ]}
-        data={users}
+        data={filteredData}
         entriesPerPage={15}
         minRows={15}
         onEntryClick={setSelectedUser}

apps/web/src/features/datahub/pages/DataHubPage.tsx

@@ -18,7 +18,6 @@ import { downloadExcel } from '@/utils/excel';
 import { MasterDataTable } from '../components/MasterDataTable';
 import { useSubjectsQuery } from '../hooks/useSubjectsQuery';
 
-// eslint-disable-next-line max-lines-per-function
 export const DataHubPage = () => {
   const [isLookupOpen, setIsLookupOpen] = useState(false);
 

apps/web/src/features/upload/components/UploadSelectTable.tsx

@@ -1,29 +1,37 @@
 import { ClientTable } from '@douglasneuroinformatics/libui/components';
 import { useTranslation } from '@douglasneuroinformatics/libui/hooks';
-import type { InstrumentInfo } from '@opendatacapture/schemas/instrument';
+import type { UnilingualInstrumentInfo } from '@opendatacapture/schemas/instrument';
 
 export type UploadSelectTableProps = {
-  data: InstrumentInfo[];
-  onSelect: (instrument: InstrumentInfo) => void;
+  data: UnilingualInstrumentInfo[];
+  onSelect: (instrument: UnilingualInstrumentInfo) => void;
 };
 
 export const UploadSelectTable = ({ data, onSelect }: UploadSelectTableProps) => {
   const { t } = useTranslation();
 
   // Renders a table for selecting an instrument to upload data for
   return (
-    <ClientTable<InstrumentInfo>
+    <ClientTable<UnilingualInstrumentInfo>
       columns={[
         {
-          field: (instrument) => (instrument.details?.title as string) ?? 'N/A',
+          field: (instrument) => instrument.details.title,
           label: t({
-            en: 'Select an instrument',
-            fr: 'Selectionnez un instrument'
+            en: 'Title',
+            fr: 'Titre'
+          })
+        },
+        {
+          field: (instrument) => instrument.kind,
+          label: t({
+            en: 'Kind',
+            fr: 'Genre'
           })
         }
       ]}
       data={data}
       entriesPerPage={15}
+      minRows={15}
       onEntryClick={onSelect}
     />
   );