From df2f14ffbfc07b7915c6e2abe0374d71c453927a Mon Sep 17 00:00:00 2001 From: Philip Molares Date: Fri, 5 Mar 2021 11:59:07 +0100 Subject: [PATCH 1/4] HistoryService: Add JSDocs for all methods Signed-off-by: Philip Molares --- src/history/history.service.ts | 50 +++++++++++++++++++++++++++++++++- 1 file changed, 49 insertions(+), 1 deletion(-) diff --git a/src/history/history.service.ts b/src/history/history.service.ts index aef7bc389..04f70eadc 100644 --- a/src/history/history.service.ts +++ b/src/history/history.service.ts @@ -29,6 +29,12 @@ export class HistoryService { this.logger.setContext(HistoryService.name); } + /** + * @async + * Get all entries of a user + * @param {User} user - the user the entries should be from + * @return {HistoryEntry[]} an array of history entries of the specified user + */ async getEntriesByUser(user: User): Promise { return await this.historyEntryRepository.find({ where: { user: user }, @@ -36,7 +42,15 @@ export class HistoryService { }); } - private async getEntryByNoteIdOrAlias( + /** + * @async + * Get a history entry by the user and note, which is specified via id or alias + * @param {string} noteIdOrAlias - the id or alias specifying the note + * @param {User} user - the user that the note belongs to + * @throws {NotInDBError} the specified note does not exist + * @return {HistoryEntry} the requested history entry + */ + async getEntryByNoteIdOrAlias( noteIdOrAlias: string, user: User, ): Promise { @@ -44,6 +58,13 @@ export class HistoryService { return await this.getEntryByNote(note, user); } + /** + * @async + * Get a history entry by the user and note + * @param {Note} note - the note that the history entry belongs to + * @param {User} user - the user that the history entry belongs to + * @return {HistoryEntry} the requested history entry + */ private async getEntryByNote(note: Note, user: User): Promise { return await this.historyEntryRepository.findOne({ where: { @@ -54,6 +75,13 @@ export class HistoryService { }); } + /** + * @async + * Create or update a history entry by the user and note. If the entry is merely updated the updatedAt date is set to the current date. + * @param {Note} note - the note that the history entry belongs to + * @param {User} user - the user that the history entry belongs to + * @return {HistoryEntry} the requested history entry + */ async createOrUpdateHistoryEntry( note: Note, user: User, @@ -67,6 +95,14 @@ export class HistoryService { return await this.historyEntryRepository.save(entry); } + /** + * @async + * Update a history entry identified by the user and a note id or alias + * @param {string} noteIdOrAlias - the note that the history entry belongs to + * @param {User} user - the user that the history entry belongs to + * @param {HistoryEntryUpdateDto} updateDto - the change that should be applied to the history entry + * @return {HistoryEntry} the requested history entry + */ async updateHistoryEntry( noteIdOrAlias: string, user: User, @@ -82,6 +118,13 @@ export class HistoryService { return await this.historyEntryRepository.save(entry); } + /** + * @async + * Delete the history entry identified by the user and a note id or alias + * @param {string} noteIdOrAlias - the note that the history entry belongs to + * @param {User} user - the user that the history entry belongs to + * @throws {NotInDBError} the specified history entry does not exist + */ async deleteHistoryEntry(noteIdOrAlias: string, user: User): Promise { const entry = await this.getEntryByNoteIdOrAlias(noteIdOrAlias, user); if (!entry) { @@ -93,6 +136,11 @@ export class HistoryService { return; } + /** + * Build HistoryEntryDto from a history entry. + * @param {HistoryEntry} entry - the history entry to use + * @return {HistoryEntryDto} the built HistoryEntryDto + */ toHistoryEntryDto(entry: HistoryEntry): HistoryEntryDto { return { identifier: entry.note.alias ? entry.note.alias : entry.note.id, From de098cf68eb71714dfbc31b0d084f08068cf6395 Mon Sep 17 00:00:00 2001 From: Philip Molares Date: Fri, 5 Mar 2021 12:01:04 +0100 Subject: [PATCH 2/4] HistoryService: Add unit test for getEntryByNoteIdOrAlias Also add extra test to deleteHistoryEntry Signed-off-by: Philip Molares --- src/history/history.service.spec.ts | 41 ++++++++++++++++++++++++++--- 1 file changed, 38 insertions(+), 3 deletions(-) diff --git a/src/history/history.service.spec.ts b/src/history/history.service.spec.ts index fe104eb1c..4a949e7db 100644 --- a/src/history/history.service.spec.ts +++ b/src/history/history.service.spec.ts @@ -111,6 +111,32 @@ describe('HistoryService', () => { }); }); + describe('getEntryByNoteIdOrAlias', () => { + const user = {} as User; + const alias = 'alias'; + describe('works', () => { + it('with history entry', async () => { + const note = Note.create(user, alias); + const historyEntry = HistoryEntry.create(user, note); + jest.spyOn(historyRepo, 'findOne').mockResolvedValueOnce(historyEntry); + jest.spyOn(noteRepo, 'findOne').mockResolvedValueOnce(note); + expect(await service.getEntryByNoteIdOrAlias(alias, user)).toEqual( + historyEntry, + ); + }); + }); + describe('fails', () => { + it('with an non-existing note', async () => { + jest.spyOn(noteRepo, 'findOne').mockResolvedValueOnce(undefined); + try { + await service.getEntryByNoteIdOrAlias(alias, {} as User); + } catch (e) { + expect(e).toBeInstanceOf(NotInDBError); + } + }); + }); + }); + describe('createOrUpdateHistoryEntry', () => { describe('works', () => { it('without an preexisting entry', async () => { @@ -221,10 +247,11 @@ describe('HistoryService', () => { ); await service.deleteHistoryEntry(alias, user); }); - + }); + describe('fails', () => { + const user = {} as User; + const alias = 'alias'; it('without an entry', async () => { - const user = {} as User; - const alias = 'alias'; const note = Note.create(user, alias); jest.spyOn(historyRepo, 'findOne').mockResolvedValueOnce(undefined); jest.spyOn(noteRepo, 'findOne').mockResolvedValueOnce(note); @@ -234,6 +261,14 @@ describe('HistoryService', () => { expect(e).toBeInstanceOf(NotInDBError); } }); + it('without a note', async () => { + jest.spyOn(noteRepo, 'findOne').mockResolvedValueOnce(undefined); + try { + await service.getEntryByNoteIdOrAlias(alias, {} as User); + } catch (e) { + expect(e).toBeInstanceOf(NotInDBError); + } + }); }); }); From d4b2dc9e4a8a3419402a33d5abf0088f87c666de Mon Sep 17 00:00:00 2001 From: Philip Molares Date: Fri, 5 Mar 2021 12:01:43 +0100 Subject: [PATCH 3/4] PublicAPI: Add /me/history/:note Signed-off-by: Philip Molares --- src/api/public/me/me.controller.ts | 24 ++++++++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/src/api/public/me/me.controller.ts b/src/api/public/me/me.controller.ts index 1169820ea..1b9e73b5f 100644 --- a/src/api/public/me/me.controller.ts +++ b/src/api/public/me/me.controller.ts @@ -59,6 +59,26 @@ export class MeController { ); } + @UseGuards(TokenAuthGuard) + @Get('history/:note') + async getHistoryEntry( + @Req() req: Request, + @Param('note') note: string, + ): Promise { + try { + const foundEntry = await this.historyService.getEntryByNoteIdOrAlias( + note, + req.user, + ); + return this.historyService.toHistoryEntryDto(foundEntry); + } catch (e) { + if (e instanceof NotInDBError) { + throw new NotFoundException(e.message); + } + throw e; + } + } + @UseGuards(TokenAuthGuard) @Put('history/:note') async updateHistoryEntry( @@ -86,13 +106,13 @@ export class MeController { @UseGuards(TokenAuthGuard) @Delete('history/:note') @HttpCode(204) - deleteHistoryEntry( + async deleteHistoryEntry( @Req() req: Request, @Param('note') note: string, ): Promise { // ToDo: Check if user is allowed to delete note try { - return this.historyService.deleteHistoryEntry(note, req.user); + await this.historyService.deleteHistoryEntry(note, req.user); } catch (e) { if (e instanceof NotInDBError) { throw new NotFoundException(e.message); From 9199bd21a19ee751015678d865c855844582f91b Mon Sep 17 00:00:00 2001 From: Philip Molares Date: Fri, 5 Mar 2021 12:04:06 +0100 Subject: [PATCH 4/4] PublicE2E: Add test for GET /me/history/{note} in me.e2e-spec.ts add test for GET /me/history/{note} add error cases to PUT /me/history/{note} and DELETE /me/history/{note} activate missing test GET /me/notes/ Signed-off-by: Philip Molares --- test/public-api/me.e2e-spec.ts | 151 +++++++++++++++++++++++---------- 1 file changed, 108 insertions(+), 43 deletions(-) diff --git a/test/public-api/me.e2e-spec.ts b/test/public-api/me.e2e-spec.ts index aa4733080..068369416 100644 --- a/test/public-api/me.e2e-spec.ts +++ b/test/public-api/me.e2e-spec.ts @@ -32,6 +32,7 @@ import { HistoryModule } from '../../src/history/history.module'; import { ConfigModule } from '@nestjs/config'; import mediaConfigMock from '../../src/config/media.config.mock'; import { User } from '../../src/users/user.entity'; +import { NoteMetadataDto } from '../../src/notes/note-metadata.dto'; // TODO Tests have to be reworked using UserService functions @@ -98,62 +99,126 @@ describe('Notes', () => { .expect('Content-Type', /json/) .expect(200); const history = response.body; + expect(history.length).toEqual(1); + const historyDto = historyService.toHistoryEntryDto(createdHistoryEntry); for (const historyEntry of history) { - if (historyEntry.identifier === 'testGetHistory') { - expect(historyEntry).toEqual(createdHistoryEntry); - } + expect(historyEntry.identifier).toEqual(historyDto.identifier); + expect(historyEntry.title).toEqual(historyDto.title); + expect(historyEntry.tags).toEqual(historyDto.tags); + expect(historyEntry.pinStatus).toEqual(historyDto.pinStatus); + expect(historyEntry.lastVisited).toEqual( + historyDto.lastVisited.toISOString(), + ); } }); - it(`PUT /me/history/{note}`, async () => { - const noteName = 'testGetNoteHistory2'; - const note = await notesService.createNote('', noteName); - await historyService.createOrUpdateHistoryEntry(note, user); - const historyEntryUpdateDto = new HistoryEntryUpdateDto(); - historyEntryUpdateDto.pinStatus = true; - const response = await request(app.getHttpServer()) - .put('/me/history/' + noteName) - .send(historyEntryUpdateDto) - .expect(200); - const history = await historyService.getEntriesByUser(user); - let historyEntry: HistoryEntryDto = response.body; - expect(historyEntry.pinStatus).toEqual(true); - historyEntry = null; - for (const e of history) { - if (e.note.alias === noteName) { - historyEntry = historyService.toHistoryEntryDto(e); - } - } - expect(historyEntry.pinStatus).toEqual(true); + describe(`GET /me/history/{note}`, () => { + it('works with an existing note', async () => { + const noteName = 'testGetNoteHistory2'; + const note = await notesService.createNote('', noteName); + const createdHistoryEntry = await historyService.createOrUpdateHistoryEntry( + note, + user, + ); + const response = await request(app.getHttpServer()) + .get(`/me/history/${noteName}`) + .expect('Content-Type', /json/) + .expect(200); + const historyEntry = response.body; + const historyEntryDto = historyService.toHistoryEntryDto( + createdHistoryEntry, + ); + expect(historyEntry.identifier).toEqual(historyEntryDto.identifier); + expect(historyEntry.title).toEqual(historyEntryDto.title); + expect(historyEntry.tags).toEqual(historyEntryDto.tags); + expect(historyEntry.pinStatus).toEqual(historyEntryDto.pinStatus); + expect(historyEntry.lastVisited).toEqual( + historyEntryDto.lastVisited.toISOString(), + ); + }); + it('fails with a non-existing note', async () => { + await request(app.getHttpServer()) + .get('/me/history/i_dont_exist') + .expect('Content-Type', /json/) + .expect(404); + }); }); - it(`DELETE /me/history/{note}`, async () => { - const noteName = 'testGetNoteHistory3'; - const note = await notesService.createNote('', noteName); - await historyService.createOrUpdateHistoryEntry(note, user); - const response = await request(app.getHttpServer()) - .delete(`/me/history/${noteName}`) - .expect(204); - expect(response.body).toEqual({}); - const history = await historyService.getEntriesByUser(user); - let historyEntry: HistoryEntry = null; - for (const e of history) { - if (e.note.alias === noteName) { - historyEntry = e; + describe(`PUT /me/history/{note}`, () => { + it('works', async () => { + const noteName = 'testGetNoteHistory3'; + const note = await notesService.createNote('', noteName); + await historyService.createOrUpdateHistoryEntry(note, user); + const historyEntryUpdateDto = new HistoryEntryUpdateDto(); + historyEntryUpdateDto.pinStatus = true; + const response = await request(app.getHttpServer()) + .put('/me/history/' + noteName) + .send(historyEntryUpdateDto) + .expect(200); + const history = await historyService.getEntriesByUser(user); + let historyEntry: HistoryEntryDto = response.body; + expect(historyEntry.pinStatus).toEqual(true); + historyEntry = null; + for (const e of history) { + if (e.note.alias === noteName) { + historyEntry = historyService.toHistoryEntryDto(e); + } } - } - return expect(historyEntry).toBeNull(); + expect(historyEntry.pinStatus).toEqual(true); + }); + it('fails with a non-existing note', async () => { + await request(app.getHttpServer()) + .put('/me/history/i_dont_exist') + .expect('Content-Type', /json/) + .expect(404); + }); }); - it.skip(`GET /me/notes/`, async () => { - // TODO use function from HistoryService to add an History Entry - await notesService.createNote('This is a test note.', 'test7'); - // usersService.getALLNotesOwnedByUser() TODO Implement function + describe(`DELETE /me/history/{note}`, () => { + it('works', async () => { + const noteName = 'testGetNoteHistory4'; + const note = await notesService.createNote('', noteName); + await historyService.createOrUpdateHistoryEntry(note, user); + const response = await request(app.getHttpServer()) + .delete(`/me/history/${noteName}`) + .expect(204); + expect(response.body).toEqual({}); + const history = await historyService.getEntriesByUser(user); + let historyEntry: HistoryEntry = null; + for (const e of history) { + if (e.note.alias === noteName) { + historyEntry = e; + } + } + return expect(historyEntry).toBeNull(); + }); + describe('fails', () => { + it('with a non-existing note', async () => { + await request(app.getHttpServer()) + .delete('/me/history/i_dont_exist') + .expect(404); + }); + it('with a non-existing history entry', async () => { + const noteName = 'testGetNoteHistory5'; + await notesService.createNote('', noteName); + await request(app.getHttpServer()) + .delete(`/me/history/${noteName}`) + .expect(404); + }); + }); + }); + + it(`GET /me/notes/`, async () => { + const noteName = 'testNote'; + await notesService.createNote('', noteName, user); const response = await request(app.getHttpServer()) .get('/me/notes/') .expect('Content-Type', /json/) .expect(200); - expect(response.body.revisions).toHaveLength(1); + const noteMetaDtos = response.body as NoteMetadataDto[]; + expect(noteMetaDtos).toHaveLength(1); + expect(noteMetaDtos[0].alias).toEqual(noteName); + expect(noteMetaDtos[0].updateUser.userName).toEqual(user.userName); }); afterAll(async () => {