feat: allow to query app settings by name (#725)

* fix: add test for feature (should fail) * feat: add option to get only settings of a certain name * chore(docs): add comments * fix: remove cancel-in-progess * fix: add type filter on appData resource * chore: remove unused variables * fix: migrations * fix: add tests for type filter in app-data * fix: add tests for review
graasp · Dec 19, 2023 · 961a8fa · 961a8fa
1 parent f85d93d
commit 961a8fa
Show file tree

Hide file tree

Showing 14 changed files with 154 additions and 20 deletions.
diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml
@@ -5,7 +5,6 @@ on: [push]
 # abort running jobs if newer version is detected on same branch
 concurrency:
   group: ${{ github.head_ref || github.ref }}
-  cancel-in-progress: true
 
 jobs:
   build-node:
@@ -61,8 +60,7 @@ jobs:
           yarn build
 
       - name: yarn check
-        run:
-          yarn check
+        run: yarn check
 
       - name: apply migrations on empty database
         env:
@@ -167,14 +165,12 @@ jobs:
           restore-keys: |
             ${{ runner.os }}-jest-
 
-
       - name: yarn install
         run: |
           yarn
 
       - name: yarn test
-        run:
-          yarn test:ci --shard=${{ matrix.shard }}
+        run: yarn test:ci --shard=${{ matrix.shard }}
         env:
           CI: true
           # random keys

diff --git a/src/migrations/1702552816415-appsetting-indexes.ts b/src/migrations/1702552816415-appsetting-indexes.ts
@@ -0,0 +1,19 @@
+import { MigrationInterface, QueryRunner } from 'typeorm';
+
+export class Migrations1702552816415 implements MigrationInterface {
+  name = 'Migrations1702552816415';
+
+  public async up(queryRunner: QueryRunner): Promise<void> {
+    await queryRunner.query(
+      `CREATE INDEX "IDX_61546c650608c1e68789c64915" ON "app_setting" ("item_id", "name") `,
+    );
+    await queryRunner.query(
+      `CREATE INDEX "IDX_6079b3bb63c13f815f7dd8d8a2" ON "app_data" ("type") `,
+    );
+  }
+
+  public async down(queryRunner: QueryRunner): Promise<void> {
+    await queryRunner.query(`DROP INDEX "public"."IDX_61546c650608c1e68789c64915"`);
+    await queryRunner.query(`DROP INDEX "public"."IDX_6079b3bb63c13f815f7dd8d8a2"`);
+  }
+}
diff --git a/src/services/item/plugins/app/appData/appData.ts b/src/services/item/plugins/app/appData/appData.ts
@@ -3,6 +3,7 @@ import {
   Column,
   CreateDateColumn,
   Entity,
+  Index,
   JoinColumn,
   ManyToOne,
   PrimaryGeneratedColumn,
@@ -18,6 +19,7 @@ import { Item } from '../../../entities/Item';
 export type Filters = {
   visibility?: AppDataVisibility;
   memberId?: Member['id'];
+  type?: string;
 };
 
 @Entity()
@@ -45,6 +47,7 @@ export class AppData extends BaseEntity {
   @JoinColumn({ name: 'member_id' })
   member: Member;
 
+  @Index()
   @Column({
     nullable: false,
     length: 25,

diff --git a/src/services/item/plugins/app/appData/index.ts b/src/services/item/plugins/app/appData/index.ts
@@ -75,12 +75,12 @@ const appDataPlugin: FastifyPluginAsync = async (fastify) => {
     );
 
     // get app data
-    fastify.get<{ Params: { itemId: string } }>(
+    fastify.get<{ Params: { itemId: string }; Querystring: { type?: string } }>(
       '/:itemId/app-data',
       { schema: getForOne },
-      async ({ authTokenSubject: requestDetails, params: { itemId } }) => {
+      async ({ authTokenSubject: requestDetails, params: { itemId }, query }) => {
         const memberId = requestDetails?.memberId;
-        return appDataService.getForItem(memberId, buildRepositories(), itemId);
+        return appDataService.getForItem(memberId, buildRepositories(), itemId, query.type);
       },
     );
 

diff --git a/src/services/item/plugins/app/appData/repository.ts b/src/services/item/plugins/app/appData/repository.ts
@@ -63,14 +63,19 @@ export const AppDataRepository = AppDataSource.getRepository(AppData).extend({
     filters: Filters = {},
     permission?: PermissionLevel,
   ): Promise<AppData[]> {
-    const { memberId } = filters;
+    const { memberId, type } = filters;
 
     const query = this.createQueryBuilder('appData')
       .leftJoinAndSelect('appData.member', 'member')
       .leftJoinAndSelect('appData.creator', 'creator')
       .leftJoinAndSelect('appData.item', 'item')
       .where('item.id = :itemId', { itemId });
 
+    // filter app data to only include requested type
+    if (type) {
+      query.andWhere('appData.type = :type', { type });
+    }
+
     // restrict app data access if user is not an admin
     if (permission !== PermissionLevel.Admin) {
       query.andWhere(

diff --git a/src/services/item/plugins/app/appData/schemas.ts b/src/services/item/plugins/app/appData/schemas.ts
@@ -73,6 +73,13 @@ const deleteOne = {
 
 const getForOne = {
   params: { $ref: 'http://graasp.org/apps/#/definitions/itemIdParam' },
+  querystring: {
+    type: 'object',
+    properties: {
+      type: { type: 'string' },
+    },
+    additionalProperties: false,
+  },
   response: {
     200: {
       type: 'array',

diff --git a/src/services/item/plugins/app/appData/service.ts b/src/services/item/plugins/app/appData/service.ts
@@ -234,7 +234,12 @@ export class AppDataService {
     return appData;
   }
 
-  async getForItem(memberId: string | undefined, repositories: Repositories, itemId: string) {
+  async getForItem(
+    memberId: string | undefined,
+    repositories: Repositories,
+    itemId: string,
+    type?: string,
+  ) {
     const { appDataRepository, memberRepository, itemRepository } = repositories;
 
     // get member
@@ -249,7 +254,7 @@ export class AppDataService {
     // posting an app data is allowed to readers
     const membership = await validatePermission(repositories, PermissionLevel.Read, member, item);
 
-    return appDataRepository.getForItem(itemId, { memberId }, membership?.permission);
+    return appDataRepository.getForItem(itemId, { memberId, type }, membership?.permission);
   }
 
   // TODO: check for many items

diff --git a/src/services/item/plugins/app/appData/test/index.test.ts b/src/services/item/plugins/app/appData/test/index.test.ts
@@ -56,7 +56,15 @@ const saveAppData = async ({
     ...defaultData,
     visibility: visibility ?? AppDataVisibility.Member,
   });
-  return [s1, s2, s3];
+  const s4 = await AppDataRepository.save({
+    item,
+    creator,
+    member: member ?? creator,
+    ...defaultData,
+    visibility: visibility ?? AppDataVisibility.Member,
+    type: 'other-type',
+  });
+  return [s1, s2, s3, s4];
 };
 
 // save apps, app data, and get token
@@ -183,6 +191,38 @@ describe('App Data Tests', () => {
         expectAppData(response.json(), appData);
       });
 
+      it('Get app data by type successfully', async () => {
+        const response = await app.inject({
+          method: HttpMethod.GET,
+          url: `${APP_ITEMS_PREFIX}/${item.id}/app-data?type=other-type`,
+          headers: {
+            Authorization: `Bearer ${token}`,
+          },
+        });
+        const appDataOfType = appData.filter((d) => d.type === 'other-type');
+        const receivedAppData = await response.json();
+        expect(appDataOfType.length).toBeGreaterThanOrEqual(1);
+        expect(response.statusCode).toEqual(StatusCodes.OK);
+        expectAppData(appDataOfType, receivedAppData);
+        expect(receivedAppData.length).toEqual(appDataOfType.length);
+      });
+
+      it('Get empty data for type that does not exist', async () => {
+        const response = await app.inject({
+          method: HttpMethod.GET,
+          url: `${APP_ITEMS_PREFIX}/${item.id}/app-data?type=impossible-type`,
+          headers: {
+            Authorization: `Bearer ${token}`,
+          },
+        });
+        const appDataOfType = appData.filter((d) => d.type === 'impossible-type');
+        const receivedAppData = await response.json();
+        expect(appDataOfType.length).toEqual(0);
+        expect(response.statusCode).toEqual(StatusCodes.OK);
+        expectAppData(appDataOfType, receivedAppData);
+        expect(receivedAppData.length).toEqual(appDataOfType.length);
+      });
+
       it('Get app data with invalid item id throws', async () => {
         const response = await app.inject({
           method: HttpMethod.GET,

diff --git a/src/services/item/plugins/app/appSetting/appSettings.ts b/src/services/item/plugins/app/appSetting/appSettings.ts
@@ -3,6 +3,7 @@ import {
   Column,
   CreateDateColumn,
   Entity,
+  Index,
   JoinColumn,
   ManyToOne,
   PrimaryGeneratedColumn,
@@ -14,6 +15,7 @@ import { Member } from '../../../../member/entities/member';
 import { Item } from '../../../entities/Item';
 
 @Entity()
+@Index(['item', 'name'])
 export class AppSetting extends BaseEntity {
   @PrimaryGeneratedColumn('uuid')
   id: string = v4();

diff --git a/src/services/item/plugins/app/appSetting/index.ts b/src/services/item/plugins/app/appSetting/index.ts
@@ -94,12 +94,12 @@ const plugin: FastifyPluginAsync = async (fastify) => {
     );
 
     // get app settings
-    fastify.get<{ Params: { itemId: string } }>(
+    fastify.get<{ Params: { itemId: string }; Querystring: { name?: string } }>(
       '/:itemId/app-settings',
       { schema: getForOne },
-      async ({ authTokenSubject: requestDetails, params: { itemId } }) => {
+      async ({ authTokenSubject: requestDetails, params: { itemId }, query: { name } }) => {
         const memberId = requestDetails?.memberId;
-        return appSettingService.getForItem(memberId, buildRepositories(), itemId);
+        return appSettingService.getForItem(memberId, buildRepositories(), itemId, name);
       },
     );
   });

diff --git a/src/services/item/plugins/app/appSetting/repository.ts b/src/services/item/plugins/app/appSetting/repository.ts
@@ -60,13 +60,14 @@ export const AppSettingRepository = AppDataSource.getRepository(AppSetting).exte
     return this.findOne({ where: { id }, relations: { creator: true, item: true } });
   },
 
-  getForItem(itemId: string): Promise<AppSetting[]> {
+  getForItem(itemId: string, name?: string): Promise<AppSetting[]> {
     if (!itemId) {
       throw new ItemNotFound(itemId);
     }
     return this.find({
       where: {
         item: { id: itemId },
+        ...(name ? { name } : undefined),
       },
       relations: { creator: true, item: true },
     });

diff --git a/src/services/item/plugins/app/appSetting/schemas.ts b/src/services/item/plugins/app/appSetting/schemas.ts
@@ -66,6 +66,13 @@ const deleteOne = {
 
 const getForOne = {
   params: { $ref: 'http://graasp.org/apps/#/definitions/itemIdParam' },
+  querystring: {
+    type: 'object',
+    properties: {
+      name: { type: 'string' },
+    },
+    additionalProperties: false,
+  },
   response: {
     200: {
       type: 'array',

diff --git a/src/services/item/plugins/app/appSetting/service.ts b/src/services/item/plugins/app/appSetting/service.ts
@@ -135,7 +135,12 @@ export class AppSettingService {
     return appSettingRepository.get(appSettingId);
   }
 
-  async getForItem(memberId: string | undefined, repositories: Repositories, itemId: string) {
+  async getForItem(
+    memberId: string | undefined,
+    repositories: Repositories,
+    itemId: string,
+    name?: string,
+  ) {
     const { appSettingRepository, memberRepository } = repositories;
 
     // get member if exists
@@ -145,7 +150,7 @@ export class AppSettingService {
     // get app setting is allowed to readers
     await this.itemService.get(member, repositories, itemId);
 
-    return appSettingRepository.getForItem(itemId);
+    return appSettingRepository.getForItem(itemId, name);
   }
 
   async copyForItem(actor: Actor, repositories: Repositories, original: Item, copy: Item) {

diff --git a/src/services/item/plugins/app/appSetting/test/index.test.ts b/src/services/item/plugins/app/appSetting/test/index.test.ts
@@ -13,6 +13,12 @@ import { setItemPublic } from '../../../itemTag/test/fixtures';
 import { setUp } from '../../test/fixtures';
 import { AppSettingRepository } from '../repository';
 
+/**
+ * Check that `expected` is contained in `values`
+ * Does not check that they match exactly !
+ * @param values values returned by the API
+ * @param expected expected values
+ */
 const expectAppSettings = (values, expected) => {
   for (const expectValue of expected) {
     const value = values.find(({ id }) => id === expectValue.id);
@@ -29,7 +35,13 @@ const saveAppSettings = async ({ item, creator }) => {
   const s1 = await AppSettingRepository.save({ item, creator, ...defaultData });
   const s2 = await AppSettingRepository.save({ item, creator, ...defaultData });
   const s3 = await AppSettingRepository.save({ item, creator, ...defaultData });
-  return [s1, s2, s3];
+  const s4 = await AppSettingRepository.save({
+    item,
+    creator,
+    ...defaultData,
+    name: 'new-setting',
+  });
+  return [s1, s2, s3, s4];
 };
 
 const setUpForAppSettings = async (
@@ -125,6 +137,38 @@ describe('Apps Settings Tests', () => {
         expectAppSettings(response.json(), appSettings);
       });
 
+      it('Get named app setting successfully', async () => {
+        const { item, appSettings, token } = await setUpForAppSettings(app, actor, actor);
+
+        const response = await app.inject({
+          method: HttpMethod.GET,
+          url: `${APP_ITEMS_PREFIX}/${item.id}/app-settings?name=new-setting`,
+          headers: {
+            Authorization: `Bearer ${token}`,
+          },
+        });
+        expect(response.statusCode).toEqual(StatusCodes.OK);
+        expectAppSettings(response.json(), [appSettings.find((s) => s.name === 'new-setting')]);
+      });
+
+      it('Get unexisting named app setting successfully', async () => {
+        const { item, appSettings, token } = await setUpForAppSettings(app, actor, actor);
+
+        const response = await app.inject({
+          method: HttpMethod.GET,
+          url: `${APP_ITEMS_PREFIX}/${item.id}/app-settings?name=no-setting`,
+          headers: {
+            Authorization: `Bearer ${token}`,
+          },
+        });
+
+        const res = await response.json();
+        const expectedData = appSettings.filter((s) => s.name === 'no-setting');
+        expect(response.statusCode).toEqual(StatusCodes.OK);
+        expect(res.length).toEqual(expectedData.length);
+        expectAppSettings(res, expectedData);
+      });
+
       it('Get app setting with invalid item id throws', async () => {
         const { token } = await setUpForAppSettings(app, actor, actor);