Skip to content

Commit 2075906

Browse files
pyphiliakimspaenleh
authored
feat: return minimal item parent (#2004)
* fix: simplify parent items * refactor: fix tests * refactor: apply comment Co-authored-by: Basile Spaenlehauer <spaenleh@gmail.com> --------- Co-authored-by: kim <kim.phanhoang@epfl.ch> Co-authored-by: Basile Spaenlehauer <spaenleh@gmail.com>
1 parent 8856134 commit 2075906

5 files changed

Lines changed: 44 additions & 82 deletions

File tree

src/services/item/item.controller.read.test.ts

Lines changed: 18 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,19 @@ jest.mock('@aws-sdk/lib-storage', () => {
4949
};
5050
});
5151

52+
export const expectParents = (
53+
data: { name: string; id: string; path: string }[],
54+
parents: { name: string; id: string; path: string }[],
55+
) => {
56+
expect(data).toHaveLength(parents.length);
57+
58+
for (let i = 0; i < parents.length; i++) {
59+
expect(data[i].id).toEqual(parents[i].id);
60+
expect(data[i].name).toEqual(parents[i].name);
61+
expect(data[i].path).toEqual(parents[i].path);
62+
}
63+
};
64+
5265
describe('Item routes tests', () => {
5366
let app: FastifyInstance;
5467

@@ -1364,63 +1377,19 @@ describe('Item routes tests', () => {
13641377
assertIsMemberForTest(actor);
13651378
mockAuthenticate(actor);
13661379

1367-
const parents = [parent, child1].map((i) =>
1368-
new ItemWrapper({ ...i, creator: null }, { permission: PermissionLevel.Admin }).packed(),
1369-
);
1380+
const parents = [parent, child1];
13701381

13711382
const response = await app.inject({
13721383
method: HttpMethod.Get,
13731384
url: `/items/${childOfChild.id}/parents`,
13741385
});
13751386

1376-
const data = response.json<PackedItem[]>();
1377-
expect(data).toHaveLength(parents.length);
1378-
data.forEach((p, idx) => {
1379-
expectPackedItem(p, parents[idx]);
1380-
expectThumbnails(p, MOCK_SIGNED_URL, false);
1381-
});
1382-
expect(response.statusCode).toBe(StatusCodes.OK);
1383-
});
1384-
1385-
it('Returns successfully with thumbnails', async () => {
1386-
const {
1387-
actor,
1388-
items: [parent, child1, childOfChild],
1389-
} = await seedFromJson({
1390-
items: [
1391-
{
1392-
settings: { hasThumbnail: true },
1393-
memberships: [{ account: 'actor', permission: PermissionLevel.Admin }],
1394-
children: [
1395-
{
1396-
settings: { hasThumbnail: true },
1397-
children: [{ settings: { hasThumbnail: true } }],
1398-
},
1399-
],
1400-
},
1401-
{},
1402-
],
1403-
});
1404-
assertIsDefined(actor);
1405-
assertIsMemberForTest(actor);
1406-
mockAuthenticate(actor);
1407-
1408-
const response = await app.inject({
1409-
method: HttpMethod.Get,
1410-
url: `/items/${childOfChild.id}/parents`,
1411-
});
1387+
const data = response.json();
14121388

1413-
const parents = [parent, child1].map((i) =>
1414-
new ItemWrapper({ ...i, creator: null }, { permission: PermissionLevel.Admin }).packed(),
1415-
);
1416-
const data = response.json<PackedItem[]>();
1417-
expect(data).toHaveLength(parents.length);
1418-
data.forEach((p, idx) => {
1419-
expectPackedItem(p, parents[idx]);
1420-
expectThumbnails(p, MOCK_SIGNED_URL, true);
1421-
});
1389+
expectParents(data, parents);
14221390
expect(response.statusCode).toBe(StatusCodes.OK);
14231391
});
1392+
14241393
it('Bad Request for invalid id', async () => {
14251394
const response = await app.inject({
14261395
method: HttpMethod.Get,
@@ -1465,7 +1434,6 @@ describe('Item routes tests', () => {
14651434
it('Returns successfully', async () => {
14661435
const {
14671436
items: [parent, child1, childOfChild],
1468-
itemVisibilities: [publicVisibility],
14691437
} = await seedFromJson({
14701438
actor: null,
14711439
items: [
@@ -1486,11 +1454,7 @@ describe('Item routes tests', () => {
14861454

14871455
const data = response.json();
14881456
expect(data).toHaveLength(parents.length);
1489-
data.forEach((p, idx) => {
1490-
expectPackedItem(p, { ...parents[idx], permission: null }, undefined, undefined, [
1491-
publicVisibility,
1492-
]);
1493-
});
1457+
expectParents(data, parents);
14941458
expect(response.statusCode).toBe(StatusCodes.OK);
14951459
});
14961460
});

src/services/item/item.controller.ts

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -13,15 +13,9 @@ import { assertIsMember } from '../authentication';
1313
import { memberAccountRole } from '../member/strategies/memberAccountRole';
1414
import { validatedMemberAccountRole } from '../member/strategies/validatedMemberAccountRole';
1515
import { ITEMS_PAGE_SIZE } from './constants';
16-
import { copyMany, deleteMany, moveMany, reorder, updateOne } from './item.schemas';
16+
import { copyMany, deleteMany, getParentItems, moveMany, reorder, updateOne } from './item.schemas';
1717
import { create, createWithThumbnail } from './item.schemas.create';
18-
import {
19-
getAccessible,
20-
getChildren,
21-
getDescendantItems,
22-
getOne,
23-
getParentItems,
24-
} from './item.schemas.packed';
18+
import { getAccessible, getChildren, getDescendantItems, getOne } from './item.schemas.packed';
2519
import { ItemService } from './item.service';
2620
import { ItemActionService } from './plugins/action/itemAction.service';
2721
import { getPostItemPayloadFromFormData } from './utils';

src/services/item/item.schemas.packed.ts

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -138,18 +138,3 @@ export const getDescendantItems = {
138138
'4xx': errorSchemaRef,
139139
},
140140
} as const satisfies FastifySchema;
141-
142-
export const getParentItems = {
143-
operationId: 'getParentItems',
144-
tags: ['item'],
145-
summary: 'Get parent items of item',
146-
description: 'Get parent items of item given its id.',
147-
148-
params: customType.StrictObject({
149-
id: customType.UUID(),
150-
}),
151-
response: {
152-
[StatusCodes.OK]: Type.Array(packedItemSchemaRef),
153-
'4xx': errorSchemaRef,
154-
},
155-
} as const satisfies FastifySchema;

src/services/item/item.schemas.ts

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,23 @@ export const itemUpdateSchema = Type.Partial(
114114
),
115115
);
116116

117+
export const getParentItems = {
118+
operationId: 'getParentItems',
119+
tags: ['item'],
120+
summary: 'Get parents of item',
121+
description: 'Get parent items of item given its id.',
122+
123+
params: customType.StrictObject({
124+
id: customType.UUID(),
125+
}),
126+
response: {
127+
[StatusCodes.OK]: Type.Array(
128+
customType.StrictObject({ id: Type.String(), name: Type.String(), path: Type.String() }),
129+
),
130+
'4xx': errorSchemaRef,
131+
},
132+
} as const satisfies FastifySchema;
133+
117134
export const updateOne = {
118135
operationId: 'updateItem',
119136
tags: ['item'],

src/services/item/item.service.ts

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -592,17 +592,19 @@ export class ItemService {
592592
});
593593
const parents = await this.itemRepository.getAncestors(dbConnection, item);
594594

595-
const { itemMemberships, visibilities } =
596-
await this.authorizedItemService.getPropertiesForItems(dbConnection, {
595+
const { itemMemberships } = await this.authorizedItemService.getPropertiesForItems(
596+
dbConnection,
597+
{
597598
permission: PermissionLevel.Read,
598599
accountId: maybeUser?.id,
599600
items: parents,
600-
});
601+
},
602+
);
601603
// remove parents actor does not have access
602604
const parentsIds = Object.keys(itemMemberships.data);
603605
const items = parents.filter((p) => parentsIds.includes(p.id));
604-
const thumbnails = await this.itemThumbnailService.getUrlsByItems(items);
605-
return this.itemWrapperService.merge(items, itemMemberships, visibilities, thumbnails);
606+
607+
return items;
606608
}
607609

608610
async patch(

0 commit comments

Comments
 (0)