fix(note): fix type for owner param

To make the create method easier to use in conjunction
with the authentication framework, this commit changes the type of
the `owner` parameter from `User | undefined` to `User | null`.

Signed-off-by: David Mehren <git@herrmehren.de>

src/api/private/notes/notes.controller.ts

@@ -91,7 +91,7 @@ export class NotesController {
     }
     this.logger.debug('Got raw markdown:\n' + text);
     return await this.noteService.toNoteDto(
-      await this.noteService.createNote(text, undefined, user),
+      await this.noteService.createNote(text, user),
     );
   }
 
@@ -108,7 +108,7 @@ export class NotesController {
     this.logger.debug('Got raw markdown:\n' + text, 'createNamedNote');
     try {
       return await this.noteService.toNoteDto(
-        await this.noteService.createNote(text, noteAlias, user),
+        await this.noteService.createNote(text, user, noteAlias),
       );
     } catch (e) {
       if (e instanceof AlreadyInDBError) {

src/api/public/notes/notes.controller.ts

@@ -93,7 +93,7 @@ export class NotesController {
     }
     this.logger.debug('Got raw markdown:\n' + text);
     return await this.noteService.toNoteDto(
-      await this.noteService.createNote(text, undefined, user),
+      await this.noteService.createNote(text, user),
     );
   }
 
@@ -135,7 +135,7 @@ export class NotesController {
     this.logger.debug('Got raw markdown:\n' + text, 'createNamedNote');
     try {
       return await this.noteService.toNoteDto(
-        await this.noteService.createNote(text, noteAlias, user),
+        await this.noteService.createNote(text, user, noteAlias),
       );
     } catch (e) {
       if (e instanceof AlreadyInDBError) {

src/notes/note.entity.ts

@@ -92,7 +92,12 @@ export class Note {
   // eslint-disable-next-line @typescript-eslint/no-empty-function
   private constructor() {}
 
-  public static create(owner?: User, alias?: string): Omit<Note, 'id'> {
+  /**
+   * Creates a new Note
+   * @param owner The owner of the note
+   * @param alias Optional primary alias
+   */
+  public static create(owner: User | null, alias?: string): Omit<Note, 'id'> {
     const newNote = new Note();
     newNote.publicId = generatePublicId();
     newNote.aliases = alias
@@ -101,7 +106,7 @@ export class Note {
     newNote.userPermissions = [];
     newNote.groupPermissions = [];
     newNote.viewCount = 0;
-    newNote.owner = owner ?? null;
+    newNote.owner = owner;
     newNote.revisions = Promise.resolve([]);
     newNote.historyEntries = [];
     newNote.mediaUploads = [];

src/notes/notes.service.spec.ts

@@ -164,7 +164,7 @@ describe('NotesService', () => {
           .mockImplementation(async (note: Note): Promise<Note> => note);
       });
       it('without alias, without owner', async () => {
-        const newNote = await service.createNote(content);
+        const newNote = await service.createNote(content, null);
         const revisions = await newNote.revisions;
         expect(revisions).toHaveLength(1);
         expect(revisions[0].content).toEqual(content);
@@ -176,7 +176,7 @@ describe('NotesService', () => {
         expect(newNote.aliases).toHaveLength(0);
       });
       it('without alias, with owner', async () => {
-        const newNote = await service.createNote(content, undefined, user);
+        const newNote = await service.createNote(content, user);
         const revisions = await newNote.revisions;
         expect(revisions).toHaveLength(1);
         expect(revisions[0].content).toEqual(content);
@@ -189,7 +189,7 @@ describe('NotesService', () => {
         expect(newNote.aliases).toHaveLength(0);
       });
       it('with alias, without owner', async () => {
-        const newNote = await service.createNote(content, alias);
+        const newNote = await service.createNote(content, null, alias);
         const revisions = await newNote.revisions;
         expect(revisions).toHaveLength(1);
         expect(revisions[0].content).toEqual(content);
@@ -201,7 +201,7 @@ describe('NotesService', () => {
         expect(newNote.aliases).toHaveLength(1);
       });
       it('with alias, with owner', async () => {
-        const newNote = await service.createNote(content, alias, user);
+        const newNote = await service.createNote(content, user, alias);
         const revisions = await newNote.revisions;
         expect(revisions).toHaveLength(1);
         expect(revisions[0].content).toEqual(content);
@@ -218,7 +218,7 @@ describe('NotesService', () => {
     describe('fails:', () => {
       it('alias is forbidden', async () => {
         await expect(
-          service.createNote(content, forbiddenNoteId),
+          service.createNote(content, null, forbiddenNoteId),
         ).rejects.toThrow(ForbiddenIdError);
       });
 
@@ -226,7 +226,7 @@ describe('NotesService', () => {
         jest.spyOn(noteRepo, 'save').mockImplementationOnce(async () => {
           throw new Error();
         });
-        await expect(service.createNote(content, alias)).rejects.toThrow(
+        await expect(service.createNote(content, null, alias)).rejects.toThrow(
           AlreadyInDBError,
         );
       });
@@ -239,7 +239,7 @@ describe('NotesService', () => {
       jest
         .spyOn(noteRepo, 'save')
         .mockImplementation(async (note: Note): Promise<Note> => note);
-      const newNote = await service.createNote(content);
+      const newNote = await service.createNote(content, null);
       const revisions = await newNote.revisions;
       jest.spyOn(revisionRepo, 'findOne').mockResolvedValueOnce(revisions[0]);
       await service.getNoteContent(newNote).then((result) => {
@@ -254,7 +254,7 @@ describe('NotesService', () => {
       jest
         .spyOn(noteRepo, 'save')
         .mockImplementation(async (note: Note): Promise<Note> => note);
-      const newNote = await service.createNote(content);
+      const newNote = await service.createNote(content, null);
       const revisions = await newNote.revisions;
       jest.spyOn(revisionRepo, 'findOne').mockResolvedValueOnce(revisions[0]);
       await service.getLatestRevision(newNote).then((result) => {
@@ -271,7 +271,7 @@ describe('NotesService', () => {
       jest
         .spyOn(noteRepo, 'save')
         .mockImplementation(async (note: Note): Promise<Note> => note);
-      const newNote = await service.createNote(content);
+      const newNote = await service.createNote(content, null);
       const revisions = await newNote.revisions;
       jest.spyOn(revisionRepo, 'findOne').mockResolvedValueOnce(revisions[0]);
       await service.getLatestRevision(newNote).then((result) => {
@@ -709,7 +709,7 @@ describe('NotesService', () => {
       jest
         .spyOn(noteRepo, 'save')
         .mockImplementation(async (note: Note): Promise<Note> => note);
-      const note = await service.createNote(content);
+      const note = await service.createNote(content, null);
       const revisions = await note.revisions;
       revisions[0].edits = [
         {
@@ -806,7 +806,7 @@ describe('NotesService', () => {
       jest
         .spyOn(noteRepo, 'save')
         .mockImplementation(async (note: Note): Promise<Note> => note);
-      const note = await service.createNote(content);
+      const note = await service.createNote(content, null);
       const revisions = await note.revisions;
       revisions[0].edits = [
         {

src/notes/notes.service.ts

@@ -88,8 +88,8 @@ export class NotesService {
    */
   async createNote(
     noteContent: string,
+    owner: User | null,
     alias?: string,
-    owner?: User,
   ): Promise<Note> {
     if (alias) {
       this.checkNoteIdOrAlias(alias);

test/private-api/alias.e2e-spec.ts

@@ -49,8 +49,8 @@ describe('Alias', () => {
     beforeAll(async () => {
       const note = await testSetup.notesService.createNote(
         content,
-        testAlias,
         user,
+        testAlias,
       );
       publicId = note.publicId;
     });
@@ -104,8 +104,8 @@ describe('Alias', () => {
     beforeAll(async () => {
       const note = await testSetup.notesService.createNote(
         content,
-        testAlias,
         user,
+        testAlias,
       );
       publicId = note.publicId;
       await testSetup.aliasService.addAlias(note, newAlias);
@@ -153,8 +153,8 @@ describe('Alias', () => {
     beforeAll(async () => {
       const note = await testSetup.notesService.createNote(
         content,
-        testAlias,
         user,
+        testAlias,
       );
       await testSetup.aliasService.addAlias(note, newAlias);
     });

test/private-api/history.e2e-spec.ts

@@ -50,8 +50,8 @@ describe('History', () => {
     user = await userService.createUser('hardcoded', 'Testy');
     await identityService.createLocalIdentity(user, 'test');
     const notesService = moduleRef.get(NotesService);
-    note = await notesService.createNote(content, 'note', user);
+    note = await notesService.createNote(content, user, 'note');
-    note2 = await notesService.createNote(content, 'note2', user);
+    note2 = await notesService.createNote(content, user, 'note2');
     agent = request.agent(testSetup.app.getHttpServer());
     await agent
       .post('/api/private/auth/local/login')

test/private-api/me.e2e-spec.ts

@@ -40,8 +40,8 @@ describe('Me', () => {
 
     content = 'This is a test note.';
     alias2 = 'note2';
-    note1 = await testSetup.notesService.createNote(content, undefined, user);
+    note1 = await testSetup.notesService.createNote(content, user);
-    note2 = await testSetup.notesService.createNote(content, alias2, user);
+    note2 = await testSetup.notesService.createNote(content, user, alias2);
     agent = request.agent(testSetup.app.getHttpServer());
     await agent
       .post('/api/private/auth/local/login')

test/private-api/media.e2e-spec.ts

@@ -40,6 +40,7 @@ describe('Media', () => {
 
     await testSetup.notesService.createNote(
       'test content',
+      null,
       'test_upload_media',
     );
     user = await testSetup.userService.createUser('hardcoded', 'Testy');
@@ -109,6 +110,7 @@ describe('Media', () => {
   it('DELETE /media/{filename}', async () => {
     const testNote = await testSetup.notesService.createNote(
       'test content',
+      null,
       'test_delete_media',
     );
     const testImage = await fs.readFile('test/private-api/fixtures/test.png');

test/private-api/notes.e2e-spec.ts

@@ -74,7 +74,7 @@ describe('Notes', () => {
   describe('GET /notes/{note}', () => {
     it('works with an existing note', async () => {
       // check if we can succefully get a note that exists
-      await testSetup.notesService.createNote(content, 'test1', user);
+      await testSetup.notesService.createNote(content, user, 'test1');
       const response = await agent
         .get('/api/private/notes/test1')
         .expect('Content-Type', /json/)
@@ -133,8 +133,8 @@ describe('Notes', () => {
         const noteId = 'test3';
         const note = await testSetup.notesService.createNote(
           content,
-          noteId,
           user,
+          noteId,
         );
         await testSetup.mediaService.saveFile(testImage, user, note);
         await agent
@@ -158,8 +158,8 @@ describe('Notes', () => {
         const noteId = 'test3a';
         const note = await testSetup.notesService.createNote(
           content,
-          noteId,
           user,
+          noteId,
         );
         const url = await testSetup.mediaService.saveFile(
           testImage,
@@ -198,7 +198,7 @@ describe('Notes', () => {
 
   describe('GET /notes/{note}/revisions', () => {
     it('works with existing alias', async () => {
-      await testSetup.notesService.createNote(content, 'test4', user);
+      await testSetup.notesService.createNote(content, user, 'test4');
       const response = await agent
         .get('/api/private/notes/test4/revisions')
         .expect('Content-Type', /json/)
@@ -226,8 +226,8 @@ describe('Notes', () => {
       const noteId = 'test8';
       const note = await testSetup.notesService.createNote(
         content,
-        noteId,
         user,
+        noteId,
       );
       await testSetup.notesService.updateNote(note, 'update');
       const responseBeforeDeleting = await agent
@@ -263,8 +263,8 @@ describe('Notes', () => {
     it('works with an existing alias', async () => {
       const note = await testSetup.notesService.createNote(
         content,
-        'test5',
         user,
+        'test5',
       );
       const revision = await testSetup.notesService.getLatestRevision(note);
       const response = await agent
@@ -293,13 +293,13 @@ describe('Notes', () => {
       const extraAlias = 'test7';
       const note1 = await testSetup.notesService.createNote(
         content,
-        alias,
         user,
+        alias,
       );
       const note2 = await testSetup.notesService.createNote(
         content,
-        extraAlias,
         user,
+        extraAlias,
       );
       const response = await agent
         .get(`/api/private/notes/${alias}/media/`)
@@ -343,8 +343,8 @@ describe('Notes', () => {
       const alias = 'test11';
       await testSetup.notesService.createNote(
         'This is a test note.',
-        alias,
         user2,
+        alias,
       );
       await agent
         .get(`/api/private/notes/${alias}/media/`)

test/public-api/alias.e2e-spec.ts

@@ -39,8 +39,8 @@ describe('Notes', () => {
     beforeAll(async () => {
       const note = await testSetup.notesService.createNote(
         content,
-        testAlias,
         user,
+        testAlias,
       );
       publicId = note.publicId;
     });
@@ -94,8 +94,8 @@ describe('Notes', () => {
     beforeAll(async () => {
       const note = await testSetup.notesService.createNote(
         content,
-        testAlias,
         user,
+        testAlias,
       );
       publicId = note.publicId;
       await testSetup.aliasService.addAlias(note, newAlias);
@@ -143,8 +143,8 @@ describe('Notes', () => {
     beforeAll(async () => {
       const note = await testSetup.notesService.createNote(
         content,
-        testAlias,
         user,
+        testAlias,
       );
       await testSetup.aliasService.addAlias(note, newAlias);
     });

test/public-api/me.e2e-spec.ts

@@ -42,7 +42,7 @@ describe('Me', () => {
 
   it(`GET /me/history`, async () => {
     const noteName = 'testGetNoteHistory1';
-    const note = await testSetup.notesService.createNote('', noteName);
+    const note = await testSetup.notesService.createNote('', null, noteName);
     const createdHistoryEntry =
       await testSetup.historyService.updateHistoryEntryTimestamp(note, user);
     const response = await request(testSetup.app.getHttpServer())
@@ -67,7 +67,7 @@ describe('Me', () => {
   describe(`GET /me/history/{note}`, () => {
     it('works with an existing note', async () => {
       const noteName = 'testGetNoteHistory2';
-      const note = await testSetup.notesService.createNote('', noteName);
+      const note = await testSetup.notesService.createNote('', null, noteName);
       const createdHistoryEntry =
         await testSetup.historyService.updateHistoryEntryTimestamp(note, user);
       const response = await request(testSetup.app.getHttpServer())
@@ -96,7 +96,7 @@ describe('Me', () => {
   describe(`PUT /me/history/{note}`, () => {
     it('works', async () => {
       const noteName = 'testGetNoteHistory3';
-      const note = await testSetup.notesService.createNote('', noteName);
+      const note = await testSetup.notesService.createNote('', null, noteName);
       await testSetup.historyService.updateHistoryEntryTimestamp(note, user);
       const historyEntryUpdateDto = new HistoryEntryUpdateDto();
       historyEntryUpdateDto.pinStatus = true;
@@ -126,7 +126,7 @@ describe('Me', () => {
   describe(`DELETE /me/history/{note}`, () => {
     it('works', async () => {
       const noteName = 'testGetNoteHistory4';
-      const note = await testSetup.notesService.createNote('', noteName);
+      const note = await testSetup.notesService.createNote('', null, noteName);
       await testSetup.historyService.updateHistoryEntryTimestamp(note, user);
       const response = await request(testSetup.app.getHttpServer())
         .delete(`/api/v2/me/history/${noteName}`)
@@ -147,7 +147,7 @@ describe('Me', () => {
       });
       it('with a non-existing history entry', async () => {
         const noteName = 'testGetNoteHistory5';
-        await testSetup.notesService.createNote('', noteName);
+        await testSetup.notesService.createNote('', null, noteName);
         await request(testSetup.app.getHttpServer())
           .delete(`/api/v2/me/history/${noteName}`)
           .expect(404);
@@ -157,7 +157,7 @@ describe('Me', () => {
 
   it(`GET /me/notes/`, async () => {
     const noteName = 'testNote';
-    await testSetup.notesService.createNote('', noteName, user);
+    await testSetup.notesService.createNote('', user, noteName);
     const response = await request(testSetup.app.getHttpServer())
       .get('/api/v2/me/notes/')
       .expect('Content-Type', /json/)
@@ -171,13 +171,13 @@ describe('Me', () => {
   it('GET /me/media', async () => {
     const note1 = await testSetup.notesService.createNote(
       'This is a test note.',
-      'test8',
       await testSetup.userService.getUserByUsername('hardcoded'),
+      'test8',
     );
     const note2 = await testSetup.notesService.createNote(
       'This is a test note.',
-      'test9',
       await testSetup.userService.getUserByUsername('hardcoded'),
+      'test9',
     );
     const httpServer = testSetup.app.getHttpServer();
     const response1 = await request(httpServer)

test/public-api/media.e2e-spec.ts

@@ -38,6 +38,7 @@ describe('Media', () => {
     user = await testSetup.userService.createUser('hardcoded', 'Testy');
     testNote = await testSetup.notesService.createNote(
       'test content',
+      null,
       'test_upload_media',
     );
   });

test/public-api/notes.e2e-spec.ts

@@ -61,7 +61,7 @@ describe('Notes', () => {
   describe('GET /notes/{note}', () => {
     it('works with an existing note', async () => {
       // check if we can succefully get a note that exists
-      await testSetup.notesService.createNote(content, 'test1', user);
+      await testSetup.notesService.createNote(content, user, 'test1');
       const response = await request(testSetup.app.getHttpServer())
         .get('/api/v2/notes/test1')
         .expect('Content-Type', /json/)
@@ -127,8 +127,8 @@ describe('Notes', () => {
         const noteId = 'test3';
         const note = await testSetup.notesService.createNote(
           content,
-          noteId,
           user,
+          noteId,
         );
         await testSetup.mediaService.saveFile(testImage, user, note);
         await request(testSetup.app.getHttpServer())
@@ -151,8 +151,8 @@ describe('Notes', () => {
         const noteId = 'test3a';
         const note = await testSetup.notesService.createNote(
           content,
-          noteId,
           user,
+          noteId,
         );
         const url = await testSetup.mediaService.saveFile(
           testImage,
@@ -183,8 +183,9 @@ describe('Notes', () => {
     it('works with an existing alias with permissions', async () => {
       const note = await testSetup.notesService.createNote(
         content,
-        'test3',
         user,
+
+        'test3',
       );
       const updateNotePermission = new NotePermissionsUpdateDto();
       updateNotePermission.sharedToUsers = [
@@ -233,7 +234,7 @@ describe('Notes', () => {
   describe('PUT /notes/{note}', () => {
     const changedContent = 'New note text';
     it('works with existing alias', async () => {
-      await testSetup.notesService.createNote(content, 'test4', user);
+      await testSetup.notesService.createNote(content, user, 'test4');
       const response = await request(testSetup.app.getHttpServer())
         .put('/api/v2/notes/test4')
         .set('Content-Type', 'text/markdown')
@@ -264,7 +265,7 @@ describe('Notes', () => {
 
   describe('GET /notes/{note}/metadata', () => {
     it('returns complete metadata object', async () => {
-      await testSetup.notesService.createNote(content, 'test5', user);
+      await testSetup.notesService.createNote(content, user, 'test5');
       const metadata = await request(testSetup.app.getHttpServer())
         .get('/api/v2/notes/test5/metadata')
         .expect(200);
@@ -306,8 +307,9 @@ describe('Notes', () => {
       // create a note
       const note = await testSetup.notesService.createNote(
         content,
-        'test5a',
         user,
+
+        'test5a',
       );
       // save the creation time
       const createDate = (await note.revisions)[0].createdAt;
@@ -325,7 +327,7 @@ describe('Notes', () => {
 
   describe('GET /notes/{note}/revisions', () => {
     it('works with existing alias', async () => {
-      await testSetup.notesService.createNote(content, 'test6', user);
+      await testSetup.notesService.createNote(content, user, 'test6');
       const response = await request(testSetup.app.getHttpServer())
         .get('/api/v2/notes/test6/revisions')
         .expect('Content-Type', /json/)
@@ -352,8 +354,9 @@ describe('Notes', () => {
     it('works with an existing alias', async () => {
       const note = await testSetup.notesService.createNote(
         content,
-        'test7',
         user,
+
+        'test7',
       );
       const revision = await testSetup.notesService.getLatestRevision(note);
       const response = await request(testSetup.app.getHttpServer())
@@ -378,7 +381,7 @@ describe('Notes', () => {
 
   describe('GET /notes/{note}/content', () => {
     it('works with an existing alias', async () => {
-      await testSetup.notesService.createNote(content, 'test8', user);
+      await testSetup.notesService.createNote(content, user, 'test8');
       const response = await request(testSetup.app.getHttpServer())
         .get('/api/v2/notes/test8/content')
         .expect(200);
@@ -404,13 +407,13 @@ describe('Notes', () => {
       const extraAlias = 'test10';
       const note1 = await testSetup.notesService.createNote(
         content,
-        alias,
         user,
+        alias,
       );
       const note2 = await testSetup.notesService.createNote(
         content,
-        extraAlias,
         user,
+        extraAlias,
       );
       const httpServer = testSetup.app.getHttpServer();
       const response = await request(httpServer)
@@ -455,8 +458,8 @@ describe('Notes', () => {
       const alias = 'test11';
       await testSetup.notesService.createNote(
         'This is a test note.',
-        alias,
         user2,
+        alias,
       );
       await request(testSetup.app.getHttpServer())
         .get(`/api/v2/notes/${alias}/media/`)