- More comprehensive testing of deleted, inactive and reassigned devi…

…ces. - Fix behaviour when re-assigning a device, so we don't orphan recordings. - Fix handling of inactive devices in browse-next.
TheCacophonyProject · Oct 15, 2024 · 0a2afbb · 0a2afbb
1 parent 814a772
commit 0a2afbb
Show file tree

Hide file tree

Showing 21 changed files with 1,064 additions and 158 deletions.
diff --git a/api/api/V1/Device.ts b/api/api/V1/Device.ts
@@ -388,6 +388,7 @@ export default function (app: Application, baseUrl: string) {
         },
       });
       if (hasRecording) {
+        logger.info("Setting device %s with recordings inactive", deviceId);
         await response.locals.device.update({
           active: false,
         });
@@ -526,8 +527,8 @@ export default function (app: Application, baseUrl: string) {
       query("view-mode").optional().equals("user"),
       deprecatedField(query("where")), // Sidekick
       anyOf(
-        query("onlyActive").optional().isBoolean().toBoolean(),
-        query("only-active").optional().isBoolean().toBoolean()
+        query("onlyActive").default(false).isBoolean().toBoolean(),
+        query("only-active").default(false).isBoolean().toBoolean()
       ),
     ]),
     fetchAuthorizedRequiredDeviceById(param("id")),
@@ -550,8 +551,8 @@ export default function (app: Application, baseUrl: string) {
       query("view-mode").optional().equals("user"),
       deprecatedField(query("where")), // Sidekick
       anyOf(
-        query("onlyActive").optional().isBoolean().toBoolean(),
-        query("only-active").optional().isBoolean().toBoolean()
+        query("onlyActive").default(false).isBoolean().toBoolean(),
+        query("only-active").default(false).isBoolean().toBoolean()
       ),
     ]),
     fetchAuthorizedRequiredDeviceById(param("id")),
@@ -1307,6 +1308,7 @@ export default function (app: Application, baseUrl: string) {
     validateFields([
       idOf(param("id")),
       body("maskRegions").custom(jsonSchemaOf(MaskRegionsSchema)),
+      booleanOf(query("only-active"), false),
     ]),
     fetchAuthorizedRequiredDeviceById(param("id")),
     async (request: Request, response: Response, next: NextFunction) => {
@@ -1415,6 +1417,7 @@ export default function (app: Application, baseUrl: string) {
     validateFields([
       idOf(param("id")),
       query("at-time").default(new Date().toISOString()).isISO8601().toDate(),
+      booleanOf(query("only-active"), false),
     ]),
     fetchAuthorizedRequiredDeviceById(param("id")),
     async (request: Request, response: Response, next: NextFunction) => {
@@ -1528,6 +1531,7 @@ export default function (app: Application, baseUrl: string) {
     validateFields([
       idOf(param("id")),
       body("settings").custom(jsonSchemaOf(ApiDeviceHistorySettingsSchema)),
+      booleanOf(query("only-active"), false),
     ]),
     fetchAuthorizedRequiredDeviceById(param("id")),
     async (request: Request, response: Response, next: NextFunction) => {
@@ -1581,6 +1585,7 @@ export default function (app: Application, baseUrl: string) {
       body("setStationAtTime").custom(
         jsonSchemaOf(ApiDeviceLocationFixupSchema)
       ),
+      booleanOf(query("only-active"), false),
     ]),
     fetchAdminAuthorizedRequiredDeviceById(param("id")),
     parseJSONField(body("setStationAtTime")),
@@ -1853,7 +1858,7 @@ export default function (app: Application, baseUrl: string) {
     validateFields([
       nameOrIdOf(param("groupIdOrName")),
       nameOf(param("deviceName")),
-      query("only-active").optional().isBoolean().toBoolean(),
+      booleanOf(query("only-active"), false),
       query("view-mode").optional().equals("user"),
     ]),
     fetchAuthorizedRequiredDeviceInGroup(
@@ -1925,7 +1930,7 @@ export default function (app: Application, baseUrl: string) {
     extractJwtAuthorizedUser,
     validateFields([
       idOf(query("deviceId")),
-      query("only-active").optional().isBoolean().toBoolean(),
+      booleanOf(query("only-active"), false),
       query("view-mode").optional().equals("user"),
     ]),
     // Should this require admin access to the device?
@@ -1939,7 +1944,7 @@ export default function (app: Application, baseUrl: string) {
     extractJwtAuthorizedUser,
     validateFields([
       idOf(param("deviceId")),
-      query("only-active").optional().isBoolean().toBoolean(),
+      booleanOf(query("only-active"), false),
       query("view-mode").optional().equals("user"),
     ]),
     // Should this require admin access to the device?
@@ -2073,8 +2078,9 @@ export default function (app: Application, baseUrl: string) {
       nameOf(body("newGroup")),
       validNameOf(body("newName")),
       validPasswordOf(body("newPassword")),
-      // NOTE: Reregister only works on currently active devices
+      // NOTE: Re-register only works on currently active devices
     ]),
+    // FIXME: Should you really be allowed to move a device into a group you aren't an admin of?
     fetchUnauthorizedRequiredGroupByNameOrId(body("newGroup")),
     async function (request: Request, response: Response, next: NextFunction) {
       const requestDevice: Device = await models.Device.findByPk(
@@ -2160,30 +2166,49 @@ export default function (app: Application, baseUrl: string) {
    */
   app.post(
     `${apiUrl}/reregister-authorized`,
+    extractJwtAuthorisedDevice,
+    extractJwtAuthorizedUserFromBody("authorizedToken"),
     validateFields([
       nameOf(body("newGroup")),
       validNameOf(body("newName")),
       validPasswordOf(body("newPassword")),
       body("authorizedToken").exists(),
     ]),
-    extractJwtAuthorisedDevice,
-    extractJwtAuthorizedUserFromBody("authorizedToken"),
     fetchAuthorizedRequiredGroupByNameOrId(body("newGroup")),
-    async function (request: Request, response: Response, next: NextFunction) {
+    async (request: Request, response: Response, next: NextFunction) => {
+      // The user should be the admin of both groups
       const requestDevice: Device = await models.Device.findByPk(
         response.locals.requestDevice.id
       );
-      const newDevice = await requestDevice.reRegister(
+      if (!requestDevice) {
+        return next(
+          new ClientError(
+            `device not found: ${response.locals.requestDevice.id}`
+          )
+        );
+      }
+      response.locals.requestDevice = requestDevice;
+      response.locals.destGroup = response.locals.group;
+      if (response.locals.group.id !== response.locals.requestDevice.GroupId) {
+        await fetchAdminAuthorizedRequiredGroupByNameOrId(
+          requestDevice.GroupId
+        )(request, response, next);
+      } else {
+        return next();
+      }
+    },
+    async function (request: Request, response: Response, next: NextFunction) {
+      const newDevice = await response.locals.requestDevice.reRegister(
         models,
         request.body.newName,
-        response.locals.group,
+        response.locals.destGroup,
         request.body.newPassword,
         true
       );
       if (newDevice === false) {
         return next(
           new ClientError(
-            `already a device in group '${response.locals.group.groupName}' with the name '${request.body.newName}'`
+            `already a device in group '${response.locals.destGroup.groupName}' with the name '${request.body.newName}'`
           )
         );
       }
@@ -2383,7 +2408,7 @@ export default function (app: Application, baseUrl: string) {
       idOf(param("deviceId")),
       query("from").isISO8601().toDate().default(new Date()),
       integerOfWithDefault(query("window-size"), 2160), // Default to a three month rolling window
-      booleanOf(query("only-active")).optional(),
+      booleanOf(query("only-active"), false),
     ]),
     fetchAuthorizedRequiredDeviceById(param("deviceId")),
     async function (request: Request, response: Response) {
@@ -2439,7 +2464,10 @@ export default function (app: Application, baseUrl: string) {
     app.get(
       `${apiUrl}/:deviceId/history`,
       extractJwtAuthorizedUser,
-      validateFields([idOf(param("deviceId"))]),
+      validateFields([
+        idOf(param("deviceId")),
+        booleanOf(query("only-active"), false),
+      ]),
       fetchAuthorizedRequiredDeviceById(param("deviceId")),
       async function (request: Request, response: Response) {
         const history = await models.DeviceHistory.findAll({

diff --git a/api/api/V1/Recording.ts b/api/api/V1/Recording.ts
@@ -2463,6 +2463,7 @@ export default (app: Application, baseUrl: string) => {
       query("sub-class-tags").default(true).isBoolean().toBoolean(),
       query("include-deleted").default(false).isBoolean().toBoolean(),
       query("time-sensitive").default(false).isBoolean().toBoolean(),
+      query("status-recordings").default(false).isBoolean().toBoolean(),
       query("tag-mode")
         .default(TagMode.Any)
         .isString()
@@ -2553,6 +2554,9 @@ export default (app: Application, baseUrl: string) => {
         const includeDeletedRecordings = query[
           "include-deleted"
         ] as unknown as boolean;
+        const statusRecordingsOnly = query[
+          "status-recordings"
+        ] as unknown as boolean;
         const locations = ((query.locations || []) as string[]).map(Number);
         const devices = ((query.devices || []) as string[]).map(Number);
         const minDuration = query.duration as unknown as number;
@@ -2631,6 +2635,7 @@ export default (app: Application, baseUrl: string) => {
             models,
             projectId,
             minDuration,
+            statusRecordingsOnly,
             includeDeletedRecordings,
             types,
             processingState,
@@ -2662,6 +2667,7 @@ export default (app: Application, baseUrl: string) => {
             models,
             projectId,
             minDuration,
+            statusRecordingsOnly,
             includeDeletedRecordings,
             types,
             processingState,
@@ -2696,11 +2702,13 @@ export default (app: Application, baseUrl: string) => {
           dateTimeMinusThreeMonths(untilDateTime)
         );
         let timeLimitReached = false;
+
         while (accumulatedRecordingIds.length < requestedLimit) {
           const recordings = await queryRecordingsInProject(
             models,
             projectId,
             minDuration,
+            statusRecordingsOnly,
             includeDeletedRecordings,
             types,
             processingState,

diff --git a/api/api/V1/recordingsBulkQueryUtil.ts b/api/api/V1/recordingsBulkQueryUtil.ts
@@ -13,6 +13,7 @@ export const getFirstPass = (
   models: ModelsDictionary,
   projectId: GroupId,
   minDuration: number,
+  statusRecordingsOnly: boolean,
   includeDeletedRecordings: boolean,
   types: RecordingType[],
   processingState: RecordingProcessingState | undefined,
@@ -57,7 +58,9 @@ export const getFirstPass = (
         : {}),
       GroupId: projectId,
       ...(types.includes(RecordingType.Audio) ? { redacted: false } : {}),
-      duration: { [Op.gte]: minDuration },
+      duration: statusRecordingsOnly
+        ? { [Op.and]: [{ [Op.lt]: 2.5 }, { [Op.gt]: 0.0 }] }
+        : { [Op.gte]: minDuration },
       [Op.and]: [
         ...(tagMode === TagMode.UnTagged
           ? [
@@ -468,6 +471,7 @@ export const queryRecordingsInProject = async (
   models: ModelsDictionary,
   projectId: GroupId,
   minDuration: number,
+  statusRecordingsOnly: boolean,
   includeDeletedRecordings: boolean,
   types: RecordingType[],
   processingState: RecordingProcessingState | undefined,
@@ -491,6 +495,7 @@ export const queryRecordingsInProject = async (
       models,
       projectId,
       minDuration,
+      statusRecordingsOnly,
       includeDeletedRecordings,
       types,
       processingState,

diff --git a/api/api/extract-middleware.ts b/api/api/extract-middleware.ts
@@ -1645,7 +1645,7 @@ export const fetchAuthorizedRequiredGroupByNameOrId = (
   );
 
 export const fetchAdminAuthorizedRequiredGroupByNameOrId = (
-  groupNameOrId: ValidationChain
+  groupNameOrId: ValidationChain | number
 ) =>
   fetchRequiredModel(
     models.Group,

diff --git a/api/api/fileUploaders/uploadGenericRecording.ts b/api/api/fileUploaders/uploadGenericRecording.ts
@@ -42,6 +42,7 @@ import type { Group } from "@models/Group.js";
 import { isLatLon } from "@models/util/validation.js";
 import { tryToMatchLocationToStationInGroup } from "@models/util/locationUtils.js";
 import { tryReadingM4aMetadata } from "@api/m4a-metadata-reader/m4a-metadata-reader.js";
+import logger from "@log";
 
 const cameraTypes = [
   RecordingType.ThermalRaw,
@@ -645,11 +646,26 @@ export const uploadGenericRecording =
       }
       let recordingDeviceUpdatePayload = {};
       if (fromDevice) {
+        let shouldSetActive = false;
+        if (!recordingDevice.active) {
+          // Check if the device has been re-assigned to another group:
+          const activeDevice = await models.Device.findOne({
+            where: {
+              saltId: recordingDevice.saltId,
+              active: true,
+            },
+          });
+          if (!activeDevice) {
+            shouldSetActive = true;
+          }
+        }
         // Set the device active and update its connection time.
         recordingDeviceUpdatePayload = {
           lastConnectionTime: new Date(),
-          active: true,
         };
+        if (shouldSetActive) {
+          (recordingDeviceUpdatePayload as any).active = true;
+        }
       } else if (
         !fromDevice &&
         (recordingTemplate.recordingDateTime >
@@ -659,7 +675,7 @@ export const uploadGenericRecording =
         let shouldSetActive = false;
         if (!recordingDevice.active) {
           // Check if the device has been re-assigned to another group:
-          const activeDevice = models.Device.findOne({
+          const activeDevice = await models.Device.findOne({
             where: {
               saltId: recordingDevice.saltId,
               active: true,