From acaefb3996db7fcfca865f444e8b6879e158ea47 Mon Sep 17 00:00:00 2001 From: David Mehren Date: Sun, 16 Jan 2022 22:19:53 +0100 Subject: [PATCH 1/6] refactor(note-permissions-dto): do not embed User objects This is part of an effort to consistently not embed User objects in API responses. Usernames are returned instead. Signed-off-by: David Mehren --- src/notes/note-permissions.dto.ts | 8 ++++---- src/notes/notes.service.spec.ts | 9 +++------ src/notes/notes.service.ts | 2 +- test/public-api/notes.e2e-spec.ts | 2 +- 4 files changed, 9 insertions(+), 12 deletions(-) diff --git a/src/notes/note-permissions.dto.ts b/src/notes/note-permissions.dto.ts index 07d1bb52d..7594c8cf2 100644 --- a/src/notes/note-permissions.dto.ts +++ b/src/notes/note-permissions.dto.ts @@ -87,12 +87,12 @@ export class NoteGroupPermissionUpdateDto { export class NotePermissionsDto { /** - * User this permission applies to + * Username of the User this permission applies to */ - @ValidateNested() - @ApiPropertyOptional({ type: UserInfoDto }) + @IsString() + @ApiPropertyOptional() @IsOptional() - owner: UserInfoDto | null; + owner: string | null; /** * List of users the note is shared with diff --git a/src/notes/notes.service.spec.ts b/src/notes/notes.service.spec.ts index 8ebd3f04d..901c1dcdb 100644 --- a/src/notes/notes.service.spec.ts +++ b/src/notes/notes.service.spec.ts @@ -686,8 +686,7 @@ describe('NotesService', () => { }, ]); const permissions = await service.toNotePermissionsDto(note); - expect(permissions.owner).not.toEqual(null); - expect(permissions.owner?.username).toEqual(user.username); + expect(permissions.owner).toEqual(user.username); expect(permissions.sharedToUsers).toHaveLength(1); expect(permissions.sharedToUsers[0].user.username).toEqual(user.username); expect(permissions.sharedToUsers[0].canEdit).toEqual(true); @@ -777,7 +776,7 @@ describe('NotesService', () => { expect(metadataDto.description).toEqual(note.description); expect(metadataDto.editedBy).toHaveLength(1); expect(metadataDto.editedBy[0]).toEqual(user.username); - expect(metadataDto.permissions.owner.username).toEqual(user.username); + expect(metadataDto.permissions.owner).toEqual(user.username); expect(metadataDto.permissions.sharedToUsers).toHaveLength(1); expect(metadataDto.permissions.sharedToUsers[0].user.username).toEqual( user.username, @@ -879,9 +878,7 @@ describe('NotesService', () => { expect(noteDto.metadata.description).toEqual(note.description); expect(noteDto.metadata.editedBy).toHaveLength(1); expect(noteDto.metadata.editedBy[0]).toEqual(user.username); - expect(noteDto.metadata.permissions.owner.username).toEqual( - user.username, - ); + expect(noteDto.metadata.permissions.owner).toEqual(user.username); expect(noteDto.metadata.permissions.sharedToUsers).toHaveLength(1); expect( noteDto.metadata.permissions.sharedToUsers[0].user.username, diff --git a/src/notes/notes.service.ts b/src/notes/notes.service.ts index c66d6102e..cfa1668b7 100644 --- a/src/notes/notes.service.ts +++ b/src/notes/notes.service.ts @@ -373,7 +373,7 @@ export class NotesService { const userPermissions = await note.userPermissions; const groupPermissions = await note.groupPermissions; return { - owner: owner ? this.usersService.toUserDto(owner) : null, + owner: owner ? owner.username : null, sharedToUsers: userPermissions.map((noteUserPermission) => ({ user: this.usersService.toUserDto(noteUserPermission.user), canEdit: noteUserPermission.canEdit, diff --git a/test/public-api/notes.e2e-spec.ts b/test/public-api/notes.e2e-spec.ts index a120ee8c3..fada69db8 100644 --- a/test/public-api/notes.e2e-spec.ts +++ b/test/public-api/notes.e2e-spec.ts @@ -276,7 +276,7 @@ describe('Notes', () => { expect(metadata.body.description).toEqual(''); expect(typeof metadata.body.createdAt).toEqual('string'); expect(metadata.body.editedBy).toEqual([]); - expect(metadata.body.permissions.owner.username).toEqual('hardcoded'); + expect(metadata.body.permissions.owner).toEqual('hardcoded'); expect(metadata.body.permissions.sharedToUsers).toEqual([]); expect(metadata.body.permissions.sharedToUsers).toEqual([]); expect(metadata.body.tags).toEqual([]); From 93301d84c76bae610a3a15098ca775fe2261913b Mon Sep 17 00:00:00 2001 From: David Mehren Date: Sun, 16 Jan 2022 22:24:54 +0100 Subject: [PATCH 2/6] refactor(note-user-permission-entry-dto): do not embed User objects This is part of an effort to consistently not embed User objects in API responses. Usernames are returned instead. Signed-off-by: David Mehren --- src/notes/note-permissions.dto.ts | 8 ++++---- src/notes/notes.service.spec.ts | 10 +++++----- src/notes/notes.service.ts | 2 +- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/notes/note-permissions.dto.ts b/src/notes/note-permissions.dto.ts index 7594c8cf2..753344c91 100644 --- a/src/notes/note-permissions.dto.ts +++ b/src/notes/note-permissions.dto.ts @@ -17,11 +17,11 @@ import { UserInfoDto } from '../users/user-info.dto'; export class NoteUserPermissionEntryDto { /** - * User this permission applies to + * Username of the User this permission applies to */ - @ValidateNested() - @ApiProperty({ type: UserInfoDto }) - user: UserInfoDto; + @IsString() + @ApiProperty() + username: string; /** * True if the user is allowed to edit the note diff --git a/src/notes/notes.service.spec.ts b/src/notes/notes.service.spec.ts index 901c1dcdb..17f7a072f 100644 --- a/src/notes/notes.service.spec.ts +++ b/src/notes/notes.service.spec.ts @@ -688,7 +688,7 @@ describe('NotesService', () => { const permissions = await service.toNotePermissionsDto(note); expect(permissions.owner).toEqual(user.username); expect(permissions.sharedToUsers).toHaveLength(1); - expect(permissions.sharedToUsers[0].user.username).toEqual(user.username); + expect(permissions.sharedToUsers[0].username).toEqual(user.username); expect(permissions.sharedToUsers[0].canEdit).toEqual(true); expect(permissions.sharedToGroups).toHaveLength(1); expect(permissions.sharedToGroups[0].group.displayName).toEqual( @@ -778,7 +778,7 @@ describe('NotesService', () => { expect(metadataDto.editedBy[0]).toEqual(user.username); expect(metadataDto.permissions.owner).toEqual(user.username); expect(metadataDto.permissions.sharedToUsers).toHaveLength(1); - expect(metadataDto.permissions.sharedToUsers[0].user.username).toEqual( + expect(metadataDto.permissions.sharedToUsers[0].username).toEqual( user.username, ); expect(metadataDto.permissions.sharedToUsers[0].canEdit).toEqual(true); @@ -880,9 +880,9 @@ describe('NotesService', () => { expect(noteDto.metadata.editedBy[0]).toEqual(user.username); expect(noteDto.metadata.permissions.owner).toEqual(user.username); expect(noteDto.metadata.permissions.sharedToUsers).toHaveLength(1); - expect( - noteDto.metadata.permissions.sharedToUsers[0].user.username, - ).toEqual(user.username); + expect(noteDto.metadata.permissions.sharedToUsers[0].username).toEqual( + user.username, + ); expect(noteDto.metadata.permissions.sharedToUsers[0].canEdit).toEqual( true, ); diff --git a/src/notes/notes.service.ts b/src/notes/notes.service.ts index cfa1668b7..e89730eea 100644 --- a/src/notes/notes.service.ts +++ b/src/notes/notes.service.ts @@ -375,7 +375,7 @@ export class NotesService { return { owner: owner ? owner.username : null, sharedToUsers: userPermissions.map((noteUserPermission) => ({ - user: this.usersService.toUserDto(noteUserPermission.user), + username: noteUserPermission.user.username, canEdit: noteUserPermission.canEdit, })), sharedToGroups: groupPermissions.map((noteGroupPermission) => ({ From fe4e3d5657685747ea06a1315ea565999837486c Mon Sep 17 00:00:00 2001 From: David Mehren Date: Sun, 16 Jan 2022 22:45:27 +0100 Subject: [PATCH 3/6] refactor(note-group-permission-entry-dto): do not embed Group objects This is part of an effort to consistently not embed Group objects in API responses. Names are returned instead. Signed-off-by: David Mehren --- src/notes/note-permissions.dto.ts | 8 ++++---- src/notes/notes.service.spec.ts | 14 +++++++------- src/notes/notes.service.ts | 2 +- 3 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/notes/note-permissions.dto.ts b/src/notes/note-permissions.dto.ts index 753344c91..43e8e728d 100644 --- a/src/notes/note-permissions.dto.ts +++ b/src/notes/note-permissions.dto.ts @@ -52,11 +52,11 @@ export class NoteUserPermissionUpdateDto { export class NoteGroupPermissionEntryDto { /** - * Group this permission applies to + * Name of the Group this permission applies to */ - @ValidateNested() - @ApiProperty({ type: GroupInfoDto }) - group: GroupInfoDto; + @IsString() + @ApiProperty() + groupName: string; /** * True if the group members are allowed to edit the note diff --git a/src/notes/notes.service.spec.ts b/src/notes/notes.service.spec.ts index 17f7a072f..66f923099 100644 --- a/src/notes/notes.service.spec.ts +++ b/src/notes/notes.service.spec.ts @@ -691,7 +691,7 @@ describe('NotesService', () => { expect(permissions.sharedToUsers[0].username).toEqual(user.username); expect(permissions.sharedToUsers[0].canEdit).toEqual(true); expect(permissions.sharedToGroups).toHaveLength(1); - expect(permissions.sharedToGroups[0].group.displayName).toEqual( + expect(permissions.sharedToGroups[0].groupName).toEqual( group.displayName, ); expect(permissions.sharedToGroups[0].canEdit).toEqual(true); @@ -783,9 +783,9 @@ describe('NotesService', () => { ); expect(metadataDto.permissions.sharedToUsers[0].canEdit).toEqual(true); expect(metadataDto.permissions.sharedToGroups).toHaveLength(1); - expect( - metadataDto.permissions.sharedToGroups[0].group.displayName, - ).toEqual(group.displayName); + expect(metadataDto.permissions.sharedToGroups[0].groupName).toEqual( + group.displayName, + ); expect(metadataDto.permissions.sharedToGroups[0].canEdit).toEqual(true); expect(metadataDto.tags).toHaveLength(1); expect(metadataDto.tags[0]).toEqual((await note.tags)[0].name); @@ -887,9 +887,9 @@ describe('NotesService', () => { true, ); expect(noteDto.metadata.permissions.sharedToGroups).toHaveLength(1); - expect( - noteDto.metadata.permissions.sharedToGroups[0].group.displayName, - ).toEqual(group.displayName); + expect(noteDto.metadata.permissions.sharedToGroups[0].groupName).toEqual( + group.displayName, + ); expect(noteDto.metadata.permissions.sharedToGroups[0].canEdit).toEqual( true, ); diff --git a/src/notes/notes.service.ts b/src/notes/notes.service.ts index e89730eea..fdfd37116 100644 --- a/src/notes/notes.service.ts +++ b/src/notes/notes.service.ts @@ -379,7 +379,7 @@ export class NotesService { canEdit: noteUserPermission.canEdit, })), sharedToGroups: groupPermissions.map((noteGroupPermission) => ({ - group: this.groupsService.toGroupDto(noteGroupPermission.group), + groupName: noteGroupPermission.group.name, canEdit: noteGroupPermission.canEdit, })), }; From cdd4f58746a21e4dc2c6515913e106c5ab63612a Mon Sep 17 00:00:00 2001 From: David Mehren Date: Sun, 16 Jan 2022 22:46:46 +0100 Subject: [PATCH 4/6] refactor(note-group-permission-update-dto): rename attribute groupName For consistency with NoteGroupPermissionEntryDto Signed-off-by: David Mehren --- src/notes/note-permissions.dto.ts | 2 +- src/notes/notes.service.spec.ts | 18 +++++++++--------- src/notes/notes.service.ts | 4 ++-- 3 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/notes/note-permissions.dto.ts b/src/notes/note-permissions.dto.ts index 43e8e728d..99922f48a 100644 --- a/src/notes/note-permissions.dto.ts +++ b/src/notes/note-permissions.dto.ts @@ -74,7 +74,7 @@ export class NoteGroupPermissionUpdateDto { */ @IsString() @ApiProperty() - groupname: string; + groupName: string; /** * True if the group members should be allowed to edit the note diff --git a/src/notes/notes.service.spec.ts b/src/notes/notes.service.spec.ts index 66f923099..84e50d02e 100644 --- a/src/notes/notes.service.spec.ts +++ b/src/notes/notes.service.spec.ts @@ -359,12 +359,12 @@ describe('NotesService', () => { userPermissionUpdate.username = 'hardcoded'; userPermissionUpdate.canEdit = true; const groupPermissionUpate = new NoteGroupPermissionUpdateDto(); - groupPermissionUpate.groupname = 'testGroup'; + groupPermissionUpate.groupName = 'testGroup'; groupPermissionUpate.canEdit = false; const user = User.create(userPermissionUpdate.username, 'Testy') as User; const group = Group.create( - groupPermissionUpate.groupname, - groupPermissionUpate.groupname, + groupPermissionUpate.groupName, + groupPermissionUpate.groupName, false, ) as Group; const note = Note.create(user) as Note; @@ -443,7 +443,7 @@ describe('NotesService', () => { }); expect(await savedNote.userPermissions).toHaveLength(0); expect((await savedNote.groupPermissions)[0].group.name).toEqual( - groupPermissionUpate.groupname, + groupPermissionUpate.groupName, ); expect((await savedNote.groupPermissions)[0].canEdit).toEqual( groupPermissionUpate.canEdit, @@ -468,7 +468,7 @@ describe('NotesService', () => { userPermissionUpdate.canEdit, ); expect((await savedNote.groupPermissions)[0].group.name).toEqual( - groupPermissionUpate.groupname, + groupPermissionUpate.groupName, ); expect((await savedNote.groupPermissions)[0].canEdit).toEqual( groupPermissionUpate.canEdit, @@ -504,7 +504,7 @@ describe('NotesService', () => { userPermissionUpdate.canEdit, ); expect((await savedNote.groupPermissions)[0].group.name).toEqual( - groupPermissionUpate.groupname, + groupPermissionUpate.groupName, ); expect((await savedNote.groupPermissions)[0].canEdit).toEqual( groupPermissionUpate.canEdit, @@ -534,7 +534,7 @@ describe('NotesService', () => { ); expect(await savedNote.userPermissions).toHaveLength(0); expect((await savedNote.groupPermissions)[0].group.name).toEqual( - groupPermissionUpate.groupname, + groupPermissionUpate.groupName, ); expect((await savedNote.groupPermissions)[0].canEdit).toEqual( groupPermissionUpate.canEdit, @@ -570,7 +570,7 @@ describe('NotesService', () => { userPermissionUpdate.canEdit, ); expect((await savedNote.groupPermissions)[0].group.name).toEqual( - groupPermissionUpate.groupname, + groupPermissionUpate.groupName, ); expect((await savedNote.groupPermissions)[0].canEdit).toEqual( groupPermissionUpate.canEdit, @@ -613,7 +613,7 @@ describe('NotesService', () => { userPermissionUpdate.canEdit, ); expect((await savedNote.groupPermissions)[0].group.name).toEqual( - groupPermissionUpate.groupname, + groupPermissionUpate.groupName, ); expect((await savedNote.groupPermissions)[0].canEdit).toEqual( groupPermissionUpate.canEdit, diff --git a/src/notes/notes.service.ts b/src/notes/notes.service.ts index fdfd37116..7d9968046 100644 --- a/src/notes/notes.service.ts +++ b/src/notes/notes.service.ts @@ -285,7 +285,7 @@ export class NotesService { ); const groups = newPermissions.sharedToGroups.map( - (groupPermission) => groupPermission.groupname, + (groupPermission) => groupPermission.groupName, ); if (checkArrayForDuplicates(users) || checkArrayForDuplicates(groups)) { @@ -318,7 +318,7 @@ export class NotesService { // Create groupPermissions for (const newGroupPermission of newPermissions.sharedToGroups) { const group = await this.groupsService.getGroupByName( - newGroupPermission.groupname, + newGroupPermission.groupName, ); const createdPermission = NoteGroupPermission.create( group, From f63c15c1abdc9692be9c9daf8c11683c6e7a7687 Mon Sep 17 00:00:00 2001 From: David Mehren Date: Sun, 16 Jan 2022 22:54:53 +0100 Subject: [PATCH 5/6] refactor(note-metadata): do not embed User objects This is part of an effort to consistently not embed User objects in API responses. Usernames are returned instead. Signed-off-by: David Mehren --- src/notes/note-metadata.dto.ts | 6 +++--- src/notes/notes.service.spec.ts | 4 ++-- src/notes/notes.service.ts | 2 +- test/public-api/me.e2e-spec.ts | 2 +- test/public-api/notes.e2e-spec.ts | 5 +---- 5 files changed, 8 insertions(+), 11 deletions(-) diff --git a/src/notes/note-metadata.dto.ts b/src/notes/note-metadata.dto.ts index e41433720..46251c9df 100644 --- a/src/notes/note-metadata.dto.ts +++ b/src/notes/note-metadata.dto.ts @@ -77,10 +77,10 @@ export class NoteMetadataDto { /** * User that last edited the note */ - @ValidateNested() - @ApiPropertyOptional({ type: UserInfoDto }) + @IsString() + @ApiPropertyOptional() @IsOptional() - updateUser: UserInfoDto | null; + updateUsername: string | null; /** * Counts how many times the published note has been viewed diff --git a/src/notes/notes.service.spec.ts b/src/notes/notes.service.spec.ts index 84e50d02e..febf0b540 100644 --- a/src/notes/notes.service.spec.ts +++ b/src/notes/notes.service.spec.ts @@ -790,7 +790,7 @@ describe('NotesService', () => { expect(metadataDto.tags).toHaveLength(1); expect(metadataDto.tags[0]).toEqual((await note.tags)[0].name); expect(metadataDto.updatedAt).toEqual(revisions[0].createdAt); - expect(metadataDto.updateUser.username).toEqual(user.username); + expect(metadataDto.updateUsername).toEqual(user.username); expect(metadataDto.viewCount).toEqual(note.viewCount); }); }); @@ -896,7 +896,7 @@ describe('NotesService', () => { expect(noteDto.metadata.tags).toHaveLength(1); expect(noteDto.metadata.tags[0]).toEqual((await note.tags)[0].name); expect(noteDto.metadata.updatedAt).toEqual(revisions[0].createdAt); - expect(noteDto.metadata.updateUser.username).toEqual(user.username); + expect(noteDto.metadata.updateUsername).toEqual(user.username); expect(noteDto.metadata.viewCount).toEqual(note.viewCount); expect(noteDto.content).toEqual(content); }); diff --git a/src/notes/notes.service.ts b/src/notes/notes.service.ts index 7d9968046..1154515ec 100644 --- a/src/notes/notes.service.ts +++ b/src/notes/notes.service.ts @@ -406,7 +406,7 @@ export class NotesService { permissions: await this.toNotePermissionsDto(note), tags: await this.toTagList(note), updatedAt: (await this.getLatestRevision(note)).createdAt, - updateUser: updateUser ? this.usersService.toUserDto(updateUser) : null, + updateUsername: updateUser ? updateUser.username : null, viewCount: note.viewCount, }; } diff --git a/test/public-api/me.e2e-spec.ts b/test/public-api/me.e2e-spec.ts index e2d42fcb1..6549191ac 100644 --- a/test/public-api/me.e2e-spec.ts +++ b/test/public-api/me.e2e-spec.ts @@ -175,7 +175,7 @@ describe('Me', () => { const noteMetaDtos = response.body as NoteMetadataDto[]; expect(noteMetaDtos).toHaveLength(1); expect(noteMetaDtos[0].primaryAlias).toEqual(noteName); - expect(noteMetaDtos[0].updateUser?.username).toEqual(user.username); + expect(noteMetaDtos[0].updateUsername).toEqual(user.username); }); it('GET /me/media', async () => { diff --git a/test/public-api/notes.e2e-spec.ts b/test/public-api/notes.e2e-spec.ts index fada69db8..15e09af4c 100644 --- a/test/public-api/notes.e2e-spec.ts +++ b/test/public-api/notes.e2e-spec.ts @@ -281,10 +281,7 @@ describe('Notes', () => { expect(metadata.body.permissions.sharedToUsers).toEqual([]); expect(metadata.body.tags).toEqual([]); expect(typeof metadata.body.updatedAt).toEqual('string'); - expect(typeof metadata.body.updateUser.displayName).toEqual('string'); - expect(typeof metadata.body.updateUser.username).toEqual('string'); - expect(typeof metadata.body.updateUser.email).toEqual('string'); - expect(typeof metadata.body.updateUser.photo).toEqual('string'); + expect(typeof metadata.body.updateUsername).toEqual('string'); expect(typeof metadata.body.viewCount).toEqual('number'); expect(metadata.body.editedBy).toEqual([]); }); From e09cdd516228d59d981909964d688d15024a6d96 Mon Sep 17 00:00:00 2001 From: David Mehren Date: Mon, 17 Jan 2022 11:58:23 +0100 Subject: [PATCH 6/6] style(note-metadata): remove unused imports Signed-off-by: David Mehren --- src/notes/note-permissions.dto.ts | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/notes/note-permissions.dto.ts b/src/notes/note-permissions.dto.ts index 99922f48a..e2d0efdad 100644 --- a/src/notes/note-permissions.dto.ts +++ b/src/notes/note-permissions.dto.ts @@ -12,9 +12,6 @@ import { ValidateNested, } from 'class-validator'; -import { GroupInfoDto } from '../groups/group-info.dto'; -import { UserInfoDto } from '../users/user-info.dto'; - export class NoteUserPermissionEntryDto { /** * Username of the User this permission applies to