Add archiveId filter to GET records

This allows users to load records by archiveID, which will return all
records in a given archive.

Issue #617 Support loading records by archive ID

Author: slifty

packages/api/docs/src/paths/record.yaml

@@ -4,11 +4,22 @@ records:
     parameters:
       - name: recordIds
         in: query
-        required: true
+        required: false
+        description: |
+          An array of record IDs to retrieve.
+          At least one of recordIds or archiveId must be provided.
         schema:
           type: array
           items:
             type: string
+      - name: archiveId
+        in: query
+        required: false
+        description: |
+          An archive ID to filter records by.
+          At least one of recordIds or archiveId must be provided.
+        schema:
+          type: string
     security:
       - {}
       - bearerHttpAuthentication: []

packages/api/src/folder/service.ts

@@ -165,6 +165,7 @@ export const getFolderChildren = async (
 		recordIds: result.rows
 			.filter((item) => item.item_type === "record")
 			.map((record) => record.id),
+		archiveId: undefined,
 		accountEmail: email,
 		shareToken,
 	});

packages/api/src/record/controller.test.ts

@@ -446,6 +446,62 @@ describe("GET /records", () => {
 		await agent.get("/api/v2/records?recordIds[]=14").expect(500);
 		expect(logger.error).toHaveBeenCalledWith(testError);
 	});
+	test("expect to return records filtered by archiveId", async () => {
+		const response = await agent
+			.get("/api/v2/records?archiveId=1")
+			.expect(200);
+		const { body: records } = response as { body: ArchiveRecord[] };
+		const recordIds = records.map((record) => record.recordId);
+		expect(recordIds).toContain("1");
+		expect(recordIds).toContain("2");
+		expect(recordIds).toContain("3");
+		expect(recordIds).toContain("8");
+		expect(recordIds).toContain("9");
+		expect(recordIds).not.toContain("4");
+		expect(recordIds).not.toContain("5");
+		expect(recordIds).not.toContain("12");
+		expect(recordIds).not.toContain("14");
+	});
+	test("expect archiveId and recordIds to act as an AND filter", async () => {
+		const response = await agent
+			.get("/api/v2/records?archiveId=1&recordIds[]=1")
+			.expect(200);
+		const { body: records } = response as { body: ArchiveRecord[] };
+		expect(records.length).toEqual(1);
+		expect(records[0]?.recordId).toEqual("1");
+	});
+	test("expect archiveId with no matching records to return empty array", async () => {
+		const response = await agent
+			.get("/api/v2/records?archiveId=9999")
+			.expect(200);
+		const { body: records } = response as { body: ArchiveRecord[] };
+		expect(records.length).toEqual(0);
+	});
+	test("expect archiveId for non-owned archive to return only public records", async () => {
+		const response = await agent
+			.get("/api/v2/records?archiveId=2")
+			.expect(200);
+		const { body: records } = response as { body: ArchiveRecord[] };
+		const recordIds = records.map((record) => record.recordId);
+		expect(recordIds).toContain("5");
+		expect(recordIds).toContain("6");
+		expect(recordIds).not.toContain("7");
+	});
+	test("expect archiveId query without recordIds still requires archiveId", async () => {
+		await agent.get("/api/v2/records").expect(400);
+	});
+	test("expect return records by archiveId for a public record when not logged in", async () => {
+		mockExtractUserEmailFromAuthToken();
+		const response = await agent
+			.get("/api/v2/records?archiveId=1")
+			.expect(200);
+		const { body: records } = response as { body: ArchiveRecord[] };
+		const recordIds = records.map((record) => record.recordId);
+		expect(recordIds).toContain("1");
+		expect(recordIds).toContain("8");
+		expect(recordIds).not.toContain("2");
+		expect(recordIds).not.toContain("3");
+	});
 });
 
 describe("PATCH /records", () => {

packages/api/src/record/controller.ts

@@ -34,6 +34,7 @@ recordController.get(
 			validateGetRecordQuery(req.query);
 			const records = await getRecordById({
 				recordIds: req.query.recordIds,
+				archiveId: req.query.archiveId,
 				accountEmail: req.body.emailFromAuthToken,
 				shareToken: req.body.shareToken,
 			});
@@ -60,6 +61,7 @@ recordController.patch(
 			const recordId = await patchRecord(req.params.recordId, req.body);
 			const record = await getRecordById({
 				recordIds: [recordId],
+				archiveId: undefined,
 				accountEmail: req.body.emailFromAuthToken,
 			});
 

packages/api/src/record/queries/get_record_by_id.sql

@@ -1,4 +1,13 @@
-WITH RECURSIVE aggregated_files AS (
+WITH RECURSIVE candidate_records AS (
+  SELECT recordid
+  FROM record
+  WHERE
+    (:recordIds::TEXT[] IS NULL OR recordid = any(:recordIds))
+    AND (:archiveId::TEXT IS NULL OR archiveid = :archiveId::INT)
+    AND status != 'status.generic.deleted'
+),
+
+aggregated_files AS (
   (SELECT
     record_file.recordid,
     array_agg(jsonb_build_object(
@@ -26,7 +35,7 @@ WITH RECURSIVE aggregated_files AS (
     ON record_file.fileid = file.fileid
   WHERE
     file.status != 'status.generic.deleted'
-    AND record_file.recordid = any(:recordIds)
+    AND record_file.recordid IN (SELECT recordid FROM candidate_records)
   GROUP BY record_file.recordid)
 ),
 
@@ -51,7 +60,7 @@ aggregated_tags AS (
   WHERE
     tag_link.reftable = 'record'
     AND tag_link.status = 'status.generic.ok'
-    AND tag_link.refid = any(:recordIds)
+    AND tag_link.refid IN (SELECT recordid FROM candidate_records)
   GROUP BY tag_link.refid
 ),
 
@@ -87,7 +96,7 @@ aggregated_shares AS (
     AND profile_item.status = 'status.generic.ok'
     AND profile_item.string1 IS NOT NULL
     AND share.status != 'status.generic.deleted'
-    AND folder_link.recordid = any(:recordIds)
+    AND folder_link.recordid IN (SELECT recordid FROM candidate_records)
   GROUP BY share.folder_linkid
 ),
 
@@ -110,7 +119,7 @@ ancestor_unrestricted_share_tokens (
         OR shareby_url.expiresdt > current_timestamp
       )
   WHERE
-    folder_link.recordid = any(:recordIds)
+    folder_link.recordid IN (SELECT recordid FROM candidate_records)
   UNION
   SELECT
     folder_link.parentfolder_linkid,
@@ -269,7 +278,7 @@ LEFT JOIN
   aggregated_ancestor_unrestricted_share_tokens
   ON record.recordid = aggregated_ancestor_unrestricted_share_tokens.recordid
 WHERE
-  record.recordid = any(:recordIds)
+  record.recordid IN (SELECT recordid FROM candidate_records)
   AND (
     record_account.primaryemail = :accountEmail
     OR share_account.primaryemail = :accountEmail

packages/api/src/record/service.ts

@@ -12,13 +12,15 @@ import { shareLinkService } from "../share_link/service";
 import type { ShareLink } from "../share_link/models";
 
 export const getRecordById = async (requestQuery: {
-	recordIds: string[];
+	recordIds: string[] | undefined;
+	archiveId: string | undefined;
 	accountEmail: string | undefined;
 	shareToken?: string | undefined;
 }): Promise<ArchiveRecord[]> => {
 	const record = await db
 		.sql<ArchiveRecordRow>("record.queries.get_record_by_id", {
-			recordIds: requestQuery.recordIds,
+			recordIds: requestQuery.recordIds ?? null,
+			archiveId: requestQuery.archiveId ?? null,
 			accountEmail: requestQuery.accountEmail,
 			shareToken: requestQuery.shareToken,
 		})
@@ -32,18 +34,24 @@ export const getRecordById = async (requestQuery: {
 		imageRatio: +(row.imageRatio ?? 0),
 	}));
 
-	// Our API contract remains that the order in which this endpoint returns
-	// records is undefined. This ordering logic is implemented as a hotfix, and
-	// should be removed when our clients no longer rely on this endpoint returning
-	// records in a particular order.
-	const recordsById = new Map<string, ArchiveRecord>();
-	records.forEach((record: ArchiveRecord) =>
-		recordsById.set(record.recordId, record),
-	);
-	const recordsInOrder = requestQuery.recordIds
-		.map((recordId: string) => recordsById.get(recordId))
-		.filter((record: ArchiveRecord | undefined) => record !== undefined);
-	return recordsInOrder;
+	if (requestQuery.recordIds !== undefined) {
+		// Our API contract remains that the order in which this endpoint returns
+		// records is undefined. This ordering logic is implemented as a hotfix, and
+		// should be removed when our clients no longer rely on this endpoint returning
+		// records in a particular order.
+		const recordsById = new Map<string, ArchiveRecord>();
+		records.forEach((archiveRecord: ArchiveRecord) =>
+			recordsById.set(archiveRecord.recordId, archiveRecord),
+		);
+		const recordsInOrder = requestQuery.recordIds
+			.map((recordId: string) => recordsById.get(recordId))
+			.filter(
+				(archiveRecord: ArchiveRecord | undefined) =>
+					archiveRecord !== undefined,
+			);
+		return recordsInOrder;
+	}
+	return records;
 };
 
 const validateCanPatchRecord = async (

packages/api/src/record/validators.ts

@@ -5,13 +5,15 @@ import { fieldsFromUserAuthentication } from "../validators";
 
 export const validateGetRecordQuery: (
 	data: unknown,
-) => asserts data is { recordIds: string[] } = (
+) => asserts data is { recordIds?: string[]; archiveId?: string } = (
 	data: unknown,
-): asserts data is { recordIds: string[] } => {
+): asserts data is { recordIds?: string[]; archiveId?: string } => {
 	const validation = Joi.object()
 		.keys({
-			recordIds: Joi.array().items(Joi.string().required()).required(),
+			recordIds: Joi.array().items(Joi.string().required()),
+			archiveId: Joi.string(),
 		})
+		.or("recordIds", "archiveId")
 		.validate(data);
 	if (validation.error !== undefined) {
 		throw validation.error;