diff --git a/src/api/public/notes/notes.controller.ts b/src/api/public/notes/notes.controller.ts index b32538d9d..42d218c13 100644 --- a/src/api/public/notes/notes.controller.ts +++ b/src/api/public/notes/notes.controller.ts @@ -129,7 +129,7 @@ export class NotesController { throw new UnauthorizedException('Deleting note denied!'); } this.logger.debug('Deleting note: ' + noteIdOrAlias, 'deleteNote'); - await this.noteService.deleteNoteByIdOrAlias(noteIdOrAlias); + await this.noteService.deleteNote(note); this.logger.debug('Successfully deleted ' + noteIdOrAlias, 'deleteNote'); return; } catch (e) { @@ -154,7 +154,7 @@ export class NotesController { } this.logger.debug('Got raw markdown:\n' + text, 'updateNote'); return this.noteService.toNoteDto( - await this.noteService.updateNoteByIdOrAlias(noteIdOrAlias, text), + await this.noteService.updateNote(note, text), ); } catch (e) { if (e instanceof NotInDBError) { @@ -196,9 +196,7 @@ export class NotesController { if (!this.permissionsService.mayRead(req.user, note)) { throw new UnauthorizedException('Reading note denied!'); } - return this.noteService.toNoteMetadataDto( - await this.noteService.getNoteByIdOrAlias(noteIdOrAlias), - ); + return this.noteService.toNoteMetadataDto(note); } catch (e) { if (e instanceof NotInDBError) { throw new NotFoundException(e.message); @@ -223,7 +221,7 @@ export class NotesController { throw new UnauthorizedException('Updating note denied!'); } return this.noteService.toNotePermissionsDto( - await this.noteService.updateNotePermissions(noteIdOrAlias, updateDto), + await this.noteService.updateNotePermissions(note, updateDto), ); } catch (e) { if (e instanceof NotInDBError) { @@ -244,9 +242,7 @@ export class NotesController { if (!this.permissionsService.mayRead(req.user, note)) { throw new UnauthorizedException('Reading note denied!'); } - const revisions = await this.revisionsService.getAllRevisions( - noteIdOrAlias, - ); + const revisions = await this.revisionsService.getAllRevisions(note); return Promise.all( revisions.map((revision) => this.revisionsService.toRevisionMetadataDto(revision), @@ -273,7 +269,7 @@ export class NotesController { throw new UnauthorizedException('Reading note denied!'); } return this.revisionsService.toRevisionDto( - await this.revisionsService.getRevision(noteIdOrAlias, revisionId), + await this.revisionsService.getRevision(note, revisionId), ); } catch (e) { if (e instanceof NotInDBError) { diff --git a/src/notes/notes.service.spec.ts b/src/notes/notes.service.spec.ts index ecdcf9aae..9bc4c7ad8 100644 --- a/src/notes/notes.service.spec.ts +++ b/src/notes/notes.service.spec.ts @@ -245,36 +245,31 @@ describe('NotesService', () => { }); }); - describe('deleteNoteByIdOrAlias', () => { + describe('deleteNote', () => { it('works', async () => { const user = User.create('hardcoded', 'Testy') as User; const note = Note.create(user); - jest.spyOn(noteRepo, 'findOne').mockResolvedValueOnce(note); jest .spyOn(noteRepo, 'remove') .mockImplementationOnce(async (entry, _) => { expect(entry).toEqual(note); return entry; }); - await service.deleteNoteByIdOrAlias('noteThatExists'); + await service.deleteNote(note); }); }); - describe('updateNoteByIdOrAlias', () => { + describe('updateNote', () => { it('works', async () => { const user = User.create('hardcoded', 'Testy') as User; const note = Note.create(user); const revisionLength = (await note.revisions).length; - jest.spyOn(noteRepo, 'findOne').mockResolvedValueOnce(note); jest .spyOn(noteRepo, 'save') .mockImplementationOnce(async (entry: Note) => { return entry; }); - const updatedNote = await service.updateNoteByIdOrAlias( - 'noteThatExists', - 'newContent', - ); + const updatedNote = await service.updateNote(note, 'newContent'); expect(await updatedNote.revisions).toHaveLength(revisionLength + 1); }); }); @@ -294,37 +289,29 @@ describe('NotesService', () => { const note = Note.create(user); describe('works', () => { it('with empty GroupPermissions and with empty UserPermissions', async () => { - jest.spyOn(noteRepo, 'findOne').mockResolvedValueOnce(note); jest .spyOn(noteRepo, 'save') .mockImplementationOnce(async (entry: Note) => { return entry; }); - const savedNote = await service.updateNotePermissions( - 'noteThatExists', - { - sharedToUsers: [], - sharedToGroups: [], - }, - ); + const savedNote = await service.updateNotePermissions(note, { + sharedToUsers: [], + sharedToGroups: [], + }); expect(savedNote.userPermissions).toHaveLength(0); expect(savedNote.groupPermissions).toHaveLength(0); }); it('with empty GroupPermissions and with new UserPermissions', async () => { - jest.spyOn(noteRepo, 'findOne').mockResolvedValueOnce(note); jest .spyOn(noteRepo, 'save') .mockImplementationOnce(async (entry: Note) => { return entry; }); jest.spyOn(userRepo, 'findOne').mockResolvedValueOnce(user); - const savedNote = await service.updateNotePermissions( - 'noteThatExists', - { - sharedToUsers: [userPermissionUpdate], - sharedToGroups: [], - }, - ); + const savedNote = await service.updateNotePermissions(note, { + sharedToUsers: [userPermissionUpdate], + sharedToGroups: [], + }); expect(savedNote.userPermissions).toHaveLength(1); expect(savedNote.userPermissions[0].user.userName).toEqual( userPermissionUpdate.username, @@ -343,22 +330,16 @@ describe('NotesService', () => { canEdit: !userPermissionUpdate.canEdit, }, ]; - jest - .spyOn(noteRepo, 'findOne') - .mockResolvedValueOnce(noteWithPreexistingPermissions); jest .spyOn(noteRepo, 'save') .mockImplementationOnce(async (entry: Note) => { return entry; }); jest.spyOn(userRepo, 'findOne').mockResolvedValueOnce(user); - const savedNote = await service.updateNotePermissions( - 'noteThatExists', - { - sharedToUsers: [userPermissionUpdate], - sharedToGroups: [], - }, - ); + const savedNote = await service.updateNotePermissions(note, { + sharedToUsers: [userPermissionUpdate], + sharedToGroups: [], + }); expect(savedNote.userPermissions).toHaveLength(1); expect(savedNote.userPermissions[0].user.userName).toEqual( userPermissionUpdate.username, @@ -369,20 +350,16 @@ describe('NotesService', () => { expect(savedNote.groupPermissions).toHaveLength(0); }); it('with new GroupPermissions and with empty UserPermissions', async () => { - jest.spyOn(noteRepo, 'findOne').mockResolvedValueOnce(note); jest .spyOn(noteRepo, 'save') .mockImplementationOnce(async (entry: Note) => { return entry; }); jest.spyOn(groupRepo, 'findOne').mockResolvedValueOnce(group); - const savedNote = await service.updateNotePermissions( - 'noteThatExists', - { - sharedToUsers: [], - sharedToGroups: [groupPermissionUpate], - }, - ); + const savedNote = await service.updateNotePermissions(note, { + sharedToUsers: [], + sharedToGroups: [groupPermissionUpate], + }); expect(savedNote.userPermissions).toHaveLength(0); expect(savedNote.groupPermissions[0].group.name).toEqual( groupPermissionUpate.groupname, @@ -392,7 +369,6 @@ describe('NotesService', () => { ); }); it('with new GroupPermissions and with new UserPermissions', async () => { - jest.spyOn(noteRepo, 'findOne').mockResolvedValueOnce(note); jest .spyOn(noteRepo, 'save') .mockImplementationOnce(async (entry: Note) => { @@ -400,13 +376,10 @@ describe('NotesService', () => { }); jest.spyOn(userRepo, 'findOne').mockResolvedValueOnce(user); jest.spyOn(groupRepo, 'findOne').mockResolvedValueOnce(group); - const savedNote = await service.updateNotePermissions( - 'noteThatExists', - { - sharedToUsers: [userPermissionUpdate], - sharedToGroups: [groupPermissionUpate], - }, - ); + const savedNote = await service.updateNotePermissions(note, { + sharedToUsers: [userPermissionUpdate], + sharedToGroups: [groupPermissionUpate], + }); expect(savedNote.userPermissions[0].user.userName).toEqual( userPermissionUpdate.username, ); @@ -429,9 +402,6 @@ describe('NotesService', () => { canEdit: !userPermissionUpdate.canEdit, }, ]; - jest - .spyOn(noteRepo, 'findOne') - .mockResolvedValueOnce(noteWithUserPermission); jest .spyOn(noteRepo, 'save') .mockImplementationOnce(async (entry: Note) => { @@ -440,7 +410,7 @@ describe('NotesService', () => { jest.spyOn(userRepo, 'findOne').mockResolvedValueOnce(user); jest.spyOn(groupRepo, 'findOne').mockResolvedValueOnce(group); const savedNote = await service.updateNotePermissions( - 'noteThatExists', + noteWithUserPermission, { sharedToUsers: [userPermissionUpdate], sharedToGroups: [groupPermissionUpate], @@ -468,9 +438,6 @@ describe('NotesService', () => { canEdit: !groupPermissionUpate.canEdit, }, ]; - jest - .spyOn(noteRepo, 'findOne') - .mockResolvedValueOnce(noteWithPreexistingPermissions); jest.spyOn(groupRepo, 'findOne').mockResolvedValueOnce(group); jest .spyOn(noteRepo, 'save') @@ -478,7 +445,7 @@ describe('NotesService', () => { return entry; }); const savedNote = await service.updateNotePermissions( - 'noteThatExists', + noteWithPreexistingPermissions, { sharedToUsers: [], sharedToGroups: [groupPermissionUpate], @@ -501,9 +468,6 @@ describe('NotesService', () => { canEdit: !groupPermissionUpate.canEdit, }, ]; - jest - .spyOn(noteRepo, 'findOne') - .mockResolvedValueOnce(noteWithPreexistingPermissions); jest .spyOn(noteRepo, 'save') .mockImplementationOnce(async (entry: Note) => { @@ -512,7 +476,7 @@ describe('NotesService', () => { jest.spyOn(userRepo, 'findOne').mockResolvedValueOnce(user); jest.spyOn(groupRepo, 'findOne').mockResolvedValueOnce(group); const savedNote = await service.updateNotePermissions( - 'noteThatExists', + noteWithPreexistingPermissions, { sharedToUsers: [userPermissionUpdate], sharedToGroups: [groupPermissionUpate], @@ -547,9 +511,6 @@ describe('NotesService', () => { canEdit: !userPermissionUpdate.canEdit, }, ]; - jest - .spyOn(noteRepo, 'findOne') - .mockResolvedValueOnce(noteWithPreexistingPermissions); jest .spyOn(noteRepo, 'save') .mockImplementationOnce(async (entry: Note) => { @@ -558,7 +519,7 @@ describe('NotesService', () => { jest.spyOn(userRepo, 'findOne').mockResolvedValueOnce(user); jest.spyOn(groupRepo, 'findOne').mockResolvedValueOnce(group); const savedNote = await service.updateNotePermissions( - 'noteThatExists', + noteWithPreexistingPermissions, { sharedToUsers: [userPermissionUpdate], sharedToGroups: [groupPermissionUpate], @@ -580,9 +541,8 @@ describe('NotesService', () => { }); describe('fails:', () => { it('userPermissions has duplicate entries', async () => { - jest.spyOn(noteRepo, 'findOne').mockResolvedValueOnce(note); try { - await service.updateNotePermissions('noteThatExists', { + await service.updateNotePermissions(note, { sharedToUsers: [userPermissionUpdate, userPermissionUpdate], sharedToGroups: [], }); @@ -592,9 +552,8 @@ describe('NotesService', () => { }); it('groupPermissions has duplicate entries', async () => { - jest.spyOn(noteRepo, 'findOne').mockResolvedValueOnce(note); try { - await service.updateNotePermissions('noteThatExists', { + await service.updateNotePermissions(note, { sharedToUsers: [], sharedToGroups: [groupPermissionUpate, groupPermissionUpate], }); @@ -604,9 +563,8 @@ describe('NotesService', () => { }); it('userPermissions and groupPermissions have duplicate entries', async () => { - jest.spyOn(noteRepo, 'findOne').mockResolvedValueOnce(note); try { - await service.updateNotePermissions('noteThatExists', { + await service.updateNotePermissions(note, { sharedToUsers: [userPermissionUpdate, userPermissionUpdate], sharedToGroups: [groupPermissionUpate, groupPermissionUpate], }); diff --git a/src/notes/notes.service.ts b/src/notes/notes.service.ts index df264c259..8c705b36b 100644 --- a/src/notes/notes.service.ts +++ b/src/notes/notes.service.ts @@ -180,29 +180,24 @@ export class NotesService { /** * @async - * Delete a note by either their id or alias. - * @param {string} noteIdOrAlias - the notes id or alias + * Delete a note + * @param {Note} note - the note to delete * @return {Note} the note, that was deleted * @throws {NotInDBError} there is no note with this id or alias */ - async deleteNoteByIdOrAlias(noteIdOrAlias: string): Promise { - const note = await this.getNoteByIdOrAlias(noteIdOrAlias); + async deleteNote(note: Note): Promise { return await this.noteRepository.remove(note); } /** * @async - * Update a notes content. The note is specified by either their id or alias. - * @param {string} noteIdOrAlias - the notes id or alias + * Update a notes content. + * @param {Note} note - the note * @param {string} noteContent - the new content * @return {Note} the note with a new revision and new content * @throws {NotInDBError} there is no note with this id or alias */ - async updateNoteByIdOrAlias( - noteIdOrAlias: string, - noteContent: string, - ): Promise { - const note = await this.getNoteByIdOrAlias(noteIdOrAlias); + async updateNote(note: Note, noteContent: string): Promise { const revisions = await note.revisions; //TODO: Calculate patch revisions.push(Revision.create(noteContent, noteContent)); @@ -212,19 +207,17 @@ export class NotesService { /** * @async - * Update a notes permissions. The note is specified by either their id or alias. - * @param {string} noteIdOrAlias - the notes id or alias + * Update a notes permissions. + * @param {Note} note - the note * @param {NotePermissionsUpdateDto} newPermissions - the permissions the not should be set to * @return {Note} the note with the new permissions * @throws {NotInDBError} there is no note with this id or alias * @throws {PermissionsUpdateInconsistentError} the new permissions specify a user or group twice. */ async updateNotePermissions( - noteIdOrAlias: string, + note: Note, newPermissions: NotePermissionsUpdateDto, ): Promise { - const note = await this.getNoteByIdOrAlias(noteIdOrAlias); - const users = newPermissions.sharedToUsers.map( (userPermission) => userPermission.username, ); diff --git a/src/revisions/revisions.service.ts b/src/revisions/revisions.service.ts index a3735337d..b9bced063 100644 --- a/src/revisions/revisions.service.ts +++ b/src/revisions/revisions.service.ts @@ -12,6 +12,7 @@ import { NotesService } from '../notes/notes.service'; import { RevisionMetadataDto } from './revision-metadata.dto'; import { RevisionDto } from './revision.dto'; import { Revision } from './revision.entity'; +import { Note } from '../notes/note.entity'; @Injectable() export class RevisionsService { @@ -24,8 +25,7 @@ export class RevisionsService { this.logger.setContext(RevisionsService.name); } - async getAllRevisions(noteIdOrAlias: string): Promise { - const note = await this.notesService.getNoteByIdOrAlias(noteIdOrAlias); + async getAllRevisions(note: Note): Promise { return await this.revisionRepository.find({ where: { note: note, @@ -33,11 +33,7 @@ export class RevisionsService { }); } - async getRevision( - noteIdOrAlias: string, - revisionId: number, - ): Promise { - const note = await this.notesService.getNoteByIdOrAlias(noteIdOrAlias); + async getRevision(note: Note, revisionId: number): Promise { return await this.revisionRepository.findOne({ where: { id: revisionId, diff --git a/test/public-api/notes.e2e-spec.ts b/test/public-api/notes.e2e-spec.ts index 3492ff6d3..97aa31705 100644 --- a/test/public-api/notes.e2e-spec.ts +++ b/test/public-api/notes.e2e-spec.ts @@ -206,7 +206,7 @@ describe('Notes', () => { // wait one second await new Promise((r) => setTimeout(r, 1000)); // update the note - await notesService.updateNoteByIdOrAlias('test5a', 'More test content'); + await notesService.updateNote(note, 'More test content'); const metadata = await request(app.getHttpServer()) .get('/notes/test5a/metadata') .expect(200);