From b896f954b92f8ed18e51431cca412438917a7a25 Mon Sep 17 00:00:00 2001 From: Philip Molares Date: Sat, 25 Sep 2021 11:50:28 +0200 Subject: [PATCH 01/12] feat: consolidate entities create This was done to give better typings to the function signatures of entities `create` methods. It also ensures that each field that should be set to `null` is set to `null` and doesn't leave that up to the typeorm handlers. See: #1641 Signed-off-by: Philip Molares --- src/auth/auth-token.entity.ts | 10 +++----- src/authors/author.entity.ts | 4 +--- src/groups/group.entity.ts | 5 ++-- src/history/history-entry.entity.ts | 20 ++++++++++------ src/identity/identity.entity.ts | 6 ++++- src/media/media-upload.entity.ts | 2 +- src/notes/alias.entity.ts | 3 ++- src/notes/note.entity.ts | 24 +++++++++++++++---- .../note-group-permission.entity.ts | 7 +++++- .../note-user-permission.entity.ts | 7 +++++- src/revisions/edit.entity.ts | 7 +++++- src/revisions/revision.entity.ts | 9 ++++++- src/seed.ts | 11 +++++---- src/users/user.entity.ts | 14 +++++++---- 14 files changed, 89 insertions(+), 40 deletions(-) diff --git a/src/auth/auth-token.entity.ts b/src/auth/auth-token.entity.ts index dc0e66aad..80d9da025 100644 --- a/src/auth/auth-token.entity.ts +++ b/src/auth/auth-token.entity.ts @@ -45,21 +45,17 @@ export class AuthToken { lastUsed: Date | null; public static create( + keyId: string, user: User, label: string, - keyId: string, accessToken: string, validUntil: Date, - ): Pick< - AuthToken, - 'user' | 'label' | 'keyId' | 'accessTokenHash' | 'createdAt' | 'validUntil' - > { + ): Omit { const newToken = new AuthToken(); + newToken.keyId = keyId; newToken.user = user; newToken.label = label; - newToken.keyId = keyId; newToken.accessTokenHash = accessToken; - newToken.createdAt = new Date(); newToken.validUntil = validUntil; newToken.lastUsed = null; return newToken; diff --git a/src/authors/author.entity.ts b/src/authors/author.entity.ts index 94e1cab5f..bc24af958 100644 --- a/src/authors/author.entity.ts +++ b/src/authors/author.entity.ts @@ -58,9 +58,7 @@ export class Author { // eslint-disable-next-line @typescript-eslint/no-empty-function private constructor() {} - public static create( - color: number, - ): Pick { + public static create(color: number): Omit { const newAuthor = new Author(); newAuthor.color = color; newAuthor.sessions = []; diff --git a/src/groups/group.entity.ts b/src/groups/group.entity.ts index 1a63224c1..083b4fb9e 100644 --- a/src/groups/group.entity.ts +++ b/src/groups/group.entity.ts @@ -43,11 +43,12 @@ export class Group { // eslint-disable-next-line @typescript-eslint/no-empty-function private constructor() {} - public static create(name: string, displayName: string): Group { + public static create(name: string, displayName: string): Omit { const newGroup = new Group(); - newGroup.special = false; // this attribute should only be true for the two special groups newGroup.name = name; newGroup.displayName = displayName; + newGroup.special = false; // this attribute should only be true for the two special groups + newGroup.members = []; return newGroup; } } diff --git a/src/history/history-entry.entity.ts b/src/history/history-entry.entity.ts index b7ab7b54b..053822312 100644 --- a/src/history/history-entry.entity.ts +++ b/src/history/history-entry.entity.ts @@ -28,15 +28,21 @@ export class HistoryEntry { @UpdateDateColumn() updatedAt: Date; - // The optional note parameter is necessary for the createNote method in the NotesService, - // as we create the note then and don't need to add it to the HistoryEntry. - public static create(user: User, note?: Note): HistoryEntry { + /** + * Create a history entry + * @param user the user the history entry is associated with + * @param note the note the history entry is associated with + * @param [pinStatus=false] if the history entry should be pinned + */ + public static create( + user: User, + note: Note, + pinStatus = false, + ): Omit { const newHistoryEntry = new HistoryEntry(); newHistoryEntry.user = user; - if (note) { - newHistoryEntry.note = note; - } - newHistoryEntry.pinStatus = false; + newHistoryEntry.note = note; + newHistoryEntry.pinStatus = pinStatus; return newHistoryEntry; } } diff --git a/src/identity/identity.entity.ts b/src/identity/identity.entity.ts index bc36e6f10..512070d3b 100644 --- a/src/identity/identity.entity.ts +++ b/src/identity/identity.entity.ts @@ -101,11 +101,15 @@ export class Identity { user: User, providerType: ProviderType, syncSource = false, - ): Identity { + ): Omit { const newIdentity = new Identity(); newIdentity.user = user; newIdentity.providerType = providerType; + newIdentity.providerName = null; newIdentity.syncSource = syncSource; + newIdentity.providerUserId = null; + newIdentity.oAuthAccessToken = null; + newIdentity.passwordHash = null; return newIdentity; } } diff --git a/src/media/media-upload.entity.ts b/src/media/media-upload.entity.ts index 7bc0bbe62..c7259335b 100644 --- a/src/media/media-upload.entity.ts +++ b/src/media/media-upload.entity.ts @@ -59,7 +59,7 @@ export class MediaUpload { extension: string, backendType: BackendType, backendData?: string, - ): MediaUpload { + ): Omit { const upload = new MediaUpload(); const randomBytes = crypto.randomBytes(16); upload.id = randomBytes.toString('hex') + '.' + extension; diff --git a/src/notes/alias.entity.ts b/src/notes/alias.entity.ts index eb7e746ce..0be2a7d89 100644 --- a/src/notes/alias.entity.ts +++ b/src/notes/alias.entity.ts @@ -54,10 +54,11 @@ export class Alias { // eslint-disable-next-line @typescript-eslint/no-empty-function private constructor() {} - static create(name: string, primary = false): Alias { + static create(name: string, note: Note, primary = false): Omit { const alias = new Alias(); alias.name = name; alias.primary = primary; + alias.note = note; return alias; } } diff --git a/src/notes/note.entity.ts b/src/notes/note.entity.ts index 23c964e78..505db302f 100644 --- a/src/notes/note.entity.ts +++ b/src/notes/note.entity.ts @@ -27,40 +27,49 @@ import { generatePublicId } from './utils'; export class Note { @PrimaryGeneratedColumn('uuid') id: string; + @Column({ type: 'text' }) publicId: string; + @OneToMany( (_) => Alias, (alias) => alias.note, { cascade: true }, // This ensures that embedded Aliases are automatically saved to the database ) aliases: Alias[]; + @OneToMany( (_) => NoteGroupPermission, (groupPermission) => groupPermission.note, { cascade: true }, // This ensures that embedded NoteGroupPermissions are automatically saved to the database ) groupPermissions: NoteGroupPermission[]; + @OneToMany( (_) => NoteUserPermission, (userPermission) => userPermission.note, { cascade: true }, // This ensures that embedded NoteUserPermission are automatically saved to the database ) userPermissions: NoteUserPermission[]; + @Column({ nullable: false, default: 0, }) viewCount: number; + @ManyToOne((_) => User, (user) => user.ownedNotes, { onDelete: 'CASCADE', // This deletes the Note, when the associated User is deleted nullable: true, }) owner: User | null; + @OneToMany((_) => Revision, (revision) => revision.note, { cascade: true }) revisions: Promise; + @OneToMany((_) => HistoryEntry, (historyEntry) => historyEntry.user) historyEntries: HistoryEntry[]; + @OneToMany((_) => MediaUpload, (mediaUpload) => mediaUpload.note) mediaUploads: MediaUpload[]; @@ -69,6 +78,7 @@ export class Note { type: 'text', }) description: string | null; + @Column({ nullable: true, type: 'text', @@ -82,15 +92,19 @@ export class Note { // eslint-disable-next-line @typescript-eslint/no-empty-function private constructor() {} - public static create(owner?: User, alias?: string): Note { + public static create(owner?: User, alias?: string): Omit { const newNote = new Note(); newNote.publicId = generatePublicId(); - newNote.aliases = alias ? [Alias.create(alias, true)] : []; - newNote.viewCount = 0; - newNote.owner = owner ?? null; + newNote.aliases = alias + ? [Alias.create(alias, newNote, true) as Alias] + : []; newNote.userPermissions = []; newNote.groupPermissions = []; - newNote.revisions = Promise.resolve([]) as Promise; + newNote.viewCount = 0; + newNote.owner = owner ?? null; + newNote.revisions = Promise.resolve([]); + newNote.historyEntries = []; + newNote.mediaUploads = []; newNote.description = null; newNote.title = null; newNote.tags = []; diff --git a/src/permissions/note-group-permission.entity.ts b/src/permissions/note-group-permission.entity.ts index 9fda06d27..2c0eb779b 100644 --- a/src/permissions/note-group-permission.entity.ts +++ b/src/permissions/note-group-permission.entity.ts @@ -28,9 +28,14 @@ export class NoteGroupPermission { // eslint-disable-next-line @typescript-eslint/no-empty-function private constructor() {} - public static create(group: Group, canEdit: boolean): NoteGroupPermission { + public static create( + group: Group, + note: Note, + canEdit: boolean, + ): NoteGroupPermission { const groupPermission = new NoteGroupPermission(); groupPermission.group = group; + groupPermission.note = note; groupPermission.canEdit = canEdit; return groupPermission; } diff --git a/src/permissions/note-user-permission.entity.ts b/src/permissions/note-user-permission.entity.ts index 6d3a637a5..c0e128f9f 100644 --- a/src/permissions/note-user-permission.entity.ts +++ b/src/permissions/note-user-permission.entity.ts @@ -28,9 +28,14 @@ export class NoteUserPermission { // eslint-disable-next-line @typescript-eslint/no-empty-function private constructor() {} - public static create(user: User, canEdit: boolean): NoteUserPermission { + public static create( + user: User, + note: Note, + canEdit: boolean, + ): NoteUserPermission { const userPermission = new NoteUserPermission(); userPermission.user = user; + userPermission.note = note; userPermission.canEdit = canEdit; return userPermission; } diff --git a/src/revisions/edit.entity.ts b/src/revisions/edit.entity.ts index 138339923..ac2a188fe 100644 --- a/src/revisions/edit.entity.ts +++ b/src/revisions/edit.entity.ts @@ -51,8 +51,13 @@ export class Edit { // eslint-disable-next-line @typescript-eslint/no-empty-function private constructor() {} - public static create(author: Author, startPos: number, endPos: number) { + public static create( + author: Author, + startPos: number, + endPos: number, + ): Omit { const newEdit = new Edit(); + newEdit.revisions = []; newEdit.author = author; newEdit.startPos = startPos; newEdit.endPos = endPos; diff --git a/src/revisions/revision.entity.ts b/src/revisions/revision.entity.ts index 3684d0e80..749a6e494 100644 --- a/src/revisions/revision.entity.ts +++ b/src/revisions/revision.entity.ts @@ -59,6 +59,7 @@ export class Revision { */ @ManyToOne((_) => Note, (note) => note.revisions, { onDelete: 'CASCADE' }) note: Note; + /** * All edit objects which are used in the revision. */ @@ -69,11 +70,17 @@ export class Revision { // eslint-disable-next-line @typescript-eslint/no-empty-function private constructor() {} - static create(content: string, patch: string): Revision { + static create( + content: string, + patch: string, + note: Note, + ): Omit { const newRevision = new Revision(); newRevision.patch = patch; newRevision.content = content; newRevision.length = content.length; + newRevision.note = note; + newRevision.edits = []; return newRevision; } } diff --git a/src/seed.ts b/src/seed.ts index 25ea740c1..b66300909 100644 --- a/src/seed.ts +++ b/src/seed.ts @@ -57,9 +57,9 @@ createConnection({ users.push(User.create('hardcoded_2', 'Test User 2')); users.push(User.create('hardcoded_3', 'Test User 3')); const notes: Note[] = []; - notes.push(Note.create(undefined, 'test')); - notes.push(Note.create(undefined, 'test2')); - notes.push(Note.create(undefined, 'test3')); + notes.push(Note.create(undefined, 'test') as Note); + notes.push(Note.create(undefined, 'test2') as Note); + notes.push(Note.create(undefined, 'test3') as Note); for (let i = 0; i < 3; i++) { const author = connection.manager.create(Author, Author.create(1)); @@ -71,8 +71,9 @@ createConnection({ const revision = Revision.create( 'This is a test note', 'This is a test note', - ); - const edit = Edit.create(author, 1, 42); + notes[i], + ) as Revision; + const edit = Edit.create(author, 1, 42) as Edit; revision.edits = [edit]; notes[i].revisions = Promise.all([revision]); notes[i].userPermissions = []; diff --git a/src/users/user.entity.ts b/src/users/user.entity.ts index 54df92695..e965d63fb 100644 --- a/src/users/user.entity.ts +++ b/src/users/user.entity.ts @@ -79,13 +79,19 @@ export class User { public static create( username: string, displayName: string, - ): Pick< - User, - 'username' | 'displayName' | 'ownedNotes' | 'authTokens' | 'identities' - > { + ): Omit { const newUser = new User(); newUser.username = username; newUser.displayName = displayName; + newUser.photo = null; + newUser.email = null; + newUser.ownedNotes = []; + newUser.authTokens = []; + newUser.identities = Promise.resolve([]); + newUser.groups = []; + newUser.historyEntries = []; + newUser.mediaUploads = []; + newUser.authors = []; return newUser; } } From d18d23cb1635fe30cfc64da76560d3d233cb7b61 Mon Sep 17 00:00:00 2001 From: Philip Molares Date: Sat, 25 Sep 2021 11:52:42 +0200 Subject: [PATCH 02/12] fix: services use the new typings from create methods Signed-off-by: Philip Molares --- src/auth/auth.service.ts | 4 ++-- src/history/history.service.ts | 2 +- src/notes/alias.service.ts | 10 +++++----- src/notes/notes.service.ts | 10 +++++++--- 4 files changed, 15 insertions(+), 11 deletions(-) diff --git a/src/auth/auth.service.ts b/src/auth/auth.service.ts index 08f0c6e8a..789e9b7fb 100644 --- a/src/auth/auth.service.ts +++ b/src/auth/auth.service.ts @@ -79,17 +79,17 @@ export class AuthService { new Date().getTime() + 2 * 365 * 24 * 60 * 60 * 1000; if (validUntil === 0 || validUntil > maximumTokenValidity) { token = AuthToken.create( + keyId, user, identifier, - keyId, accessToken, new Date(maximumTokenValidity), ); } else { token = AuthToken.create( + keyId, user, identifier, - keyId, accessToken, new Date(validUntil), ); diff --git a/src/history/history.service.ts b/src/history/history.service.ts index 7285b06c1..a9b478b34 100644 --- a/src/history/history.service.ts +++ b/src/history/history.service.ts @@ -173,7 +173,7 @@ export class HistoryService { `Note with id/alias '${historyEntry.note}' not found.`, ); } - const entry = HistoryEntry.create(user, note); + const entry = HistoryEntry.create(user, note) as HistoryEntry; entry.pinStatus = historyEntry.pinStatus; entry.updatedAt = historyEntry.lastVisited; await manager.save(entry); diff --git a/src/notes/alias.service.ts b/src/notes/alias.service.ts index d507864c2..1d5b952b3 100644 --- a/src/notes/alias.service.ts +++ b/src/notes/alias.service.ts @@ -61,17 +61,17 @@ export class AliasService { `The alias '${alias}' is already a public id.`, ); } - let newAlias: Alias; + let newAlias; if (note.aliases.length === 0) { // the first alias is automatically made the primary alias - newAlias = Alias.create(alias, true); + newAlias = Alias.create(alias, note, true); } else { - newAlias = Alias.create(alias); + newAlias = Alias.create(alias, note); } - note.aliases.push(newAlias); + note.aliases.push(newAlias as Alias); await this.noteRepository.save(note); - return newAlias; + return newAlias as Alias; } /** diff --git a/src/notes/notes.service.ts b/src/notes/notes.service.ts index 9423cc046..5ea656cd2 100644 --- a/src/notes/notes.service.ts +++ b/src/notes/notes.service.ts @@ -97,10 +97,12 @@ export class NotesService { const newNote = Note.create(owner, alias); //TODO: Calculate patch newNote.revisions = Promise.resolve([ - Revision.create(noteContent, noteContent), + Revision.create(noteContent, noteContent, newNote as Note) as Revision, ]); if (owner) { - newNote.historyEntries = [HistoryEntry.create(owner)]; + newNote.historyEntries = [ + HistoryEntry.create(owner, newNote as Note) as HistoryEntry, + ]; } try { return await this.noteRepository.save(newNote); @@ -258,7 +260,7 @@ export class NotesService { async updateNote(note: Note, noteContent: string): Promise { const revisions = await note.revisions; //TODO: Calculate patch - revisions.push(Revision.create(noteContent, noteContent)); + revisions.push(Revision.create(noteContent, noteContent, note) as Revision); note.revisions = Promise.resolve(revisions); note.userPermissions = []; note.groupPermissions = []; @@ -306,6 +308,7 @@ export class NotesService { ); const createdPermission = NoteUserPermission.create( user, + note, newUserPermission.canEdit, ); createdPermission.note = note; @@ -319,6 +322,7 @@ export class NotesService { ); const createdPermission = NoteGroupPermission.create( group, + note, newGroupPermission.canEdit, ); createdPermission.note = note; From 58d8ff71fed64e8e1bdfcab53225e3db92b1601a Mon Sep 17 00:00:00 2001 From: Philip Molares Date: Sat, 25 Sep 2021 11:55:35 +0200 Subject: [PATCH 03/12] fix: the tests use the new typing from create methods Signed-off-by: Philip Molares --- src/auth/auth.service.spec.ts | 2 +- src/groups/groups.service.spec.ts | 2 +- src/history/history.service.spec.ts | 33 +++++++++++---------- src/history/utils.spec.ts | 4 +-- src/identity/identity.service.spec.ts | 2 +- src/media/media.service.spec.ts | 8 ++--- src/notes/alias.service.spec.ts | 27 +++++++++-------- src/notes/notes.service.spec.ts | 29 +++++++++--------- src/notes/utils.spec.ts | 4 +-- src/permissions/permissions.service.spec.ts | 20 ++++++++----- src/revisions/revisions.service.spec.ts | 11 +++---- 11 files changed, 77 insertions(+), 65 deletions(-) diff --git a/src/auth/auth.service.spec.ts b/src/auth/auth.service.spec.ts index ccb6db507..21a39d752 100644 --- a/src/auth/auth.service.spec.ts +++ b/src/auth/auth.service.spec.ts @@ -62,9 +62,9 @@ describe('AuthService', () => { user = User.create('hardcoded', 'Testy') as User; authToken = AuthToken.create( + 'testKeyId', user, 'testToken', - 'testKeyId', 'abc', new Date(new Date().getTime() + 60000), // make this AuthToken valid for 1min ) as AuthToken; diff --git a/src/groups/groups.service.spec.ts b/src/groups/groups.service.spec.ts index 62c0559c7..1a776bed2 100644 --- a/src/groups/groups.service.spec.ts +++ b/src/groups/groups.service.spec.ts @@ -39,7 +39,7 @@ describe('GroupsService', () => { service = module.get(GroupsService); groupRepo = module.get>(getRepositoryToken(Group)); - group = Group.create('testGroup', 'Superheros'); + group = Group.create('testGroup', 'Superheros') as Group; }); it('should be defined', () => { diff --git a/src/history/history.service.spec.ts b/src/history/history.service.spec.ts index c4b4c521f..bf4a02f68 100644 --- a/src/history/history.service.spec.ts +++ b/src/history/history.service.spec.ts @@ -144,7 +144,10 @@ describe('HistoryService', () => { describe('works', () => { const user = {} as User; const alias = 'alias'; - const historyEntry = HistoryEntry.create(user, Note.create(user, alias)); + const historyEntry = HistoryEntry.create( + user, + Note.create(user, alias) as Note, + ) as HistoryEntry; it('without an preexisting entry', async () => { jest.spyOn(historyRepo, 'findOne').mockResolvedValueOnce(undefined); jest @@ -153,7 +156,7 @@ describe('HistoryService', () => { async (entry: HistoryEntry): Promise => entry, ); const createHistoryEntry = await service.updateHistoryEntryTimestamp( - Note.create(user, alias), + Note.create(user, alias) as Note, user, ); expect(createHistoryEntry.note.aliases).toHaveLength(1); @@ -171,7 +174,7 @@ describe('HistoryService', () => { async (entry: HistoryEntry): Promise => entry, ); const createHistoryEntry = await service.updateHistoryEntryTimestamp( - Note.create(user, alias), + Note.create(user, alias) as Note, user, ); expect(createHistoryEntry.note.aliases).toHaveLength(1); @@ -189,7 +192,7 @@ describe('HistoryService', () => { describe('updateHistoryEntry', () => { const user = {} as User; const alias = 'alias'; - const note = Note.create(user, alias); + const note = Note.create(user, alias) as Note; beforeEach(() => { const createQueryBuilder = { leftJoinAndSelect: () => createQueryBuilder, @@ -206,7 +209,7 @@ describe('HistoryService', () => { }); describe('works', () => { it('with an entry', async () => { - const historyEntry = HistoryEntry.create(user, note); + const historyEntry = HistoryEntry.create(user, note) as HistoryEntry; jest.spyOn(historyRepo, 'findOne').mockResolvedValueOnce(historyEntry); jest .spyOn(historyRepo, 'save') @@ -242,8 +245,8 @@ describe('HistoryService', () => { describe('works', () => { const user = {} as User; const alias = 'alias'; - const note = Note.create(user, alias); - const historyEntry = HistoryEntry.create(user, note); + const note = Note.create(user, alias) as Note; + const historyEntry = HistoryEntry.create(user, note) as HistoryEntry; it('with an entry', async () => { jest.spyOn(historyRepo, 'find').mockResolvedValueOnce([historyEntry]); jest @@ -258,8 +261,8 @@ describe('HistoryService', () => { }); it('with multiple entries', async () => { const alias2 = 'alias2'; - const note2 = Note.create(user, alias2); - const historyEntry2 = HistoryEntry.create(user, note2); + const note2 = Note.create(user, alias2) as Note; + const historyEntry2 = HistoryEntry.create(user, note2) as HistoryEntry; jest .spyOn(historyRepo, 'find') .mockResolvedValueOnce([historyEntry, historyEntry2]); @@ -292,8 +295,8 @@ describe('HistoryService', () => { it('with an entry', async () => { const user = {} as User; const alias = 'alias'; - const note = Note.create(user, alias); - const historyEntry = HistoryEntry.create(user, note); + const note = Note.create(user, alias) as Note; + const historyEntry = HistoryEntry.create(user, note) as HistoryEntry; jest.spyOn(historyRepo, 'findOne').mockResolvedValueOnce(historyEntry); const createQueryBuilder = { leftJoinAndSelect: () => createQueryBuilder, @@ -322,7 +325,7 @@ describe('HistoryService', () => { const user = {} as User; const alias = 'alias'; it('without an entry', async () => { - const note = Note.create(user, alias); + const note = Note.create(user, alias) as Note; const createQueryBuilder = { leftJoinAndSelect: () => createQueryBuilder, where: () => createQueryBuilder, @@ -347,7 +350,7 @@ describe('HistoryService', () => { it('works', async () => { const user = {} as User; const alias = 'alias'; - const note = Note.create(user, alias); + const note = Note.create(user, alias) as Note; const historyEntry = HistoryEntry.create(user, note); const historyEntryImport: HistoryEntryImportDto = { lastVisited: new Date('2020-12-01 12:23:34'), @@ -397,14 +400,14 @@ describe('HistoryService', () => { const alias = 'alias'; const title = 'title'; const tags = ['tag1', 'tag2']; - const note = Note.create(user, alias); + const note = Note.create(user, alias) as Note; note.title = title; note.tags = tags.map((tag) => { const newTag = new Tag(); newTag.name = tag; return newTag; }); - const historyEntry = HistoryEntry.create(user, note); + const historyEntry = HistoryEntry.create(user, note) as HistoryEntry; historyEntry.pinStatus = true; const createQueryBuilder = { leftJoinAndSelect: () => createQueryBuilder, diff --git a/src/history/utils.spec.ts b/src/history/utils.spec.ts index d6465c27d..582d7d403 100644 --- a/src/history/utils.spec.ts +++ b/src/history/utils.spec.ts @@ -15,8 +15,8 @@ describe('getIdentifier', () => { let entry: HistoryEntry; beforeEach(() => { const user = User.create('hardcoded', 'Testy') as User; - note = Note.create(user, alias); - entry = HistoryEntry.create(user, note); + note = Note.create(user, alias) as Note; + entry = HistoryEntry.create(user, note) as HistoryEntry; }); it('returns the publicId if there are no aliases', () => { note.aliases = undefined as unknown as Alias[]; diff --git a/src/identity/identity.service.spec.ts b/src/identity/identity.service.spec.ts index 687b3d122..654a0bea3 100644 --- a/src/identity/identity.service.spec.ts +++ b/src/identity/identity.service.spec.ts @@ -95,7 +95,7 @@ describe('IdentityService', () => { describe('loginWithLocalIdentity', () => { it('works', async () => { - const identity = Identity.create(user, ProviderType.LOCAL); + const identity = Identity.create(user, ProviderType.LOCAL) as Identity; identity.passwordHash = await hashPassword(password); user.identities = Promise.resolve([identity]); await expect( diff --git a/src/media/media.service.spec.ts b/src/media/media.service.spec.ts index e5010132d..a22e6c374 100644 --- a/src/media/media.service.spec.ts +++ b/src/media/media.service.spec.ts @@ -106,7 +106,7 @@ describe('MediaService', () => { beforeEach(() => { user = User.create('hardcoded', 'Testy') as User; const alias = 'alias'; - note = Note.create(user, alias); + note = Note.create(user, alias) as Note; jest.spyOn(userRepo, 'findOne').mockResolvedValueOnce(user); const createQueryBuilder = { leftJoinAndSelect: () => createQueryBuilder, @@ -289,12 +289,12 @@ describe('MediaService', () => { describe('removeNoteFromMediaUpload', () => { it('works', async () => { + const mockNote = {} as Note; + mockNote.aliases = [Alias.create('test', mockNote, true) as Alias]; const mockMediaUploadEntry = { id: 'testMediaUpload', backendData: 'testBackendData', - note: { - aliases: [Alias.create('test', true)], - } as Note, + note: mockNote, user: { username: 'hardcoded', } as User, diff --git a/src/notes/alias.service.spec.ts b/src/notes/alias.service.spec.ts index 20bc41274..5db8679e3 100644 --- a/src/notes/alias.service.spec.ts +++ b/src/notes/alias.service.spec.ts @@ -113,7 +113,7 @@ describe('AliasService', () => { const user = User.create('hardcoded', 'Testy') as User; describe('creates', () => { it('an primary alias if no alias is already present', async () => { - const note = Note.create(user); + const note = Note.create(user) as Note; jest .spyOn(noteRepo, 'save') .mockImplementationOnce(async (note: Note): Promise => note); @@ -124,7 +124,7 @@ describe('AliasService', () => { expect(savedAlias.primary).toBeTruthy(); }); it('an non-primary alias if an primary alias is already present', async () => { - const note = Note.create(user, alias); + const note = Note.create(user, alias) as Note; jest .spyOn(noteRepo, 'save') .mockImplementationOnce(async (note: Note): Promise => note); @@ -136,11 +136,11 @@ describe('AliasService', () => { }); }); describe('does not create an alias', () => { - const note = Note.create(user, alias2); + const note = Note.create(user, alias2) as Note; it('with an already used name', async () => { jest .spyOn(aliasRepo, 'findOne') - .mockResolvedValueOnce(Alias.create(alias2)); + .mockResolvedValueOnce(Alias.create(alias2, note) as Alias); await expect(service.addAlias(note, alias2)).rejects.toThrow( AlreadyInDBError, ); @@ -158,8 +158,8 @@ describe('AliasService', () => { const alias2 = 'testAlias2'; const user = User.create('hardcoded', 'Testy') as User; describe('removes one alias correctly', () => { - const note = Note.create(user, alias); - note.aliases.push(Alias.create(alias2)); + const note = Note.create(user, alias) as Note; + note.aliases.push(Alias.create(alias2, note) as Alias); it('with two aliases', async () => { jest .spyOn(noteRepo, 'save') @@ -188,8 +188,8 @@ describe('AliasService', () => { }); }); describe('does not remove one alias', () => { - const note = Note.create(user, alias); - note.aliases.push(Alias.create(alias2)); + const note = Note.create(user, alias) as Note; + note.aliases.push(Alias.create(alias2, note) as Alias); it('if the alias is unknown', async () => { await expect(service.removeAlias(note, 'non existent')).rejects.toThrow( NotInDBError, @@ -204,10 +204,11 @@ describe('AliasService', () => { }); describe('makeAliasPrimary', () => { - const alias = Alias.create('testAlias', true); - const alias2 = Alias.create('testAlias2'); const user = User.create('hardcoded', 'Testy') as User; - const note = Note.create(user, alias.name); + const aliasName = 'testAlias'; + const note = Note.create(user, aliasName) as Note; + const alias = Alias.create(aliasName, note, true) as Alias; + const alias2 = Alias.create('testAlias2', note) as Alias; note.aliases.push(alias2); it('mark the alias as primary', async () => { jest @@ -256,9 +257,9 @@ describe('AliasService', () => { it('toAliasDto correctly creates an AliasDto', () => { const aliasName = 'testAlias'; - const alias = Alias.create(aliasName, true); const user = User.create('hardcoded', 'Testy') as User; - const note = Note.create(user, alias.name); + const note = Note.create(user, aliasName) as Note; + const alias = Alias.create(aliasName, note, true) as Alias; const aliasDto = service.toAliasDto(alias, note); expect(aliasDto.name).toEqual(aliasName); expect(aliasDto.primaryAlias).toBeTruthy(); diff --git a/src/notes/notes.service.spec.ts b/src/notes/notes.service.spec.ts index 80711f773..53ed25221 100644 --- a/src/notes/notes.service.spec.ts +++ b/src/notes/notes.service.spec.ts @@ -131,7 +131,7 @@ describe('NotesService', () => { describe('works', () => { const user = User.create('hardcoded', 'Testy') as User; const alias = 'alias'; - const note = Note.create(user, alias); + const note = Note.create(user, alias) as Note; it('with no note', async () => { jest.spyOn(noteRepo, 'find').mockResolvedValueOnce(undefined); @@ -168,7 +168,7 @@ describe('NotesService', () => { const revisions = await newNote.revisions; expect(revisions).toHaveLength(1); expect(revisions[0].content).toEqual(content); - expect(newNote.historyEntries).toBeUndefined(); + expect(newNote.historyEntries).toHaveLength(0); expect(newNote.userPermissions).toHaveLength(0); expect(newNote.groupPermissions).toHaveLength(0); expect(newNote.tags).toHaveLength(0); @@ -193,7 +193,7 @@ describe('NotesService', () => { const revisions = await newNote.revisions; expect(revisions).toHaveLength(1); expect(revisions[0].content).toEqual(content); - expect(newNote.historyEntries).toBeUndefined(); + expect(newNote.historyEntries).toHaveLength(0); expect(newNote.userPermissions).toHaveLength(0); expect(newNote.groupPermissions).toHaveLength(0); expect(newNote.tags).toHaveLength(0); @@ -328,7 +328,7 @@ describe('NotesService', () => { describe('deleteNote', () => { it('works', async () => { const user = User.create('hardcoded', 'Testy') as User; - const note = Note.create(user); + const note = Note.create(user) as Note; jest .spyOn(noteRepo, 'remove') .mockImplementationOnce(async (entry, _) => { @@ -342,7 +342,7 @@ describe('NotesService', () => { describe('updateNote', () => { it('works', async () => { const user = User.create('hardcoded', 'Testy') as User; - const note = Note.create(user); + const note = Note.create(user) as Note; const revisionLength = (await note.revisions).length; jest .spyOn(noteRepo, 'save') @@ -365,8 +365,8 @@ describe('NotesService', () => { const group = Group.create( groupPermissionUpate.groupname, groupPermissionUpate.groupname, - ); - const note = Note.create(user); + ) as Group; + const note = Note.create(user) as Note; describe('works', () => { it('with empty GroupPermissions and with empty UserPermissions', async () => { jest @@ -668,8 +668,8 @@ describe('NotesService', () => { describe('toNotePermissionsDto', () => { it('works', async () => { const user = User.create('hardcoded', 'Testy') as User; - const group = Group.create('testGroup', 'testGroup'); - const note = Note.create(user); + const group = Group.create('testGroup', 'testGroup') as Group; + const note = Note.create(user) as Note; note.userPermissions = [ { note: note, @@ -685,7 +685,8 @@ describe('NotesService', () => { }, ]; const permissions = service.toNotePermissionsDto(note); - expect(permissions.owner.username).toEqual(user.username); + expect(permissions.owner).not.toEqual(null); + expect(permissions.owner?.username).toEqual(user.username); expect(permissions.sharedToUsers).toHaveLength(1); expect(permissions.sharedToUsers[0].user.username).toEqual(user.username); expect(permissions.sharedToUsers[0].canEdit).toEqual(true); @@ -702,7 +703,7 @@ describe('NotesService', () => { const user = User.create('hardcoded', 'Testy') as User; const author = Author.create(1); author.user = user; - const group = Group.create('testGroup', 'testGroup'); + const group = Group.create('testGroup', 'testGroup') as Group; const content = 'testContent'; jest .spyOn(noteRepo, 'save') @@ -738,7 +739,7 @@ describe('NotesService', () => { // @ts-ignore .mockImplementation(() => createQueryBuilder); note.publicId = 'testId'; - note.aliases = [Alias.create('testAlias', true)]; + note.aliases = [Alias.create('testAlias', note, true) as Alias]; note.title = 'testTitle'; note.description = 'testDescription'; note.owner = user; @@ -799,7 +800,7 @@ describe('NotesService', () => { author.user = user; const otherUser = User.create('other hardcoded', 'Testy2') as User; otherUser.username = 'other hardcoded user'; - const group = Group.create('testGroup', 'testGroup'); + const group = Group.create('testGroup', 'testGroup') as Group; const content = 'testContent'; jest .spyOn(noteRepo, 'save') @@ -838,7 +839,7 @@ describe('NotesService', () => { // @ts-ignore .mockImplementation(() => createQueryBuilder); note.publicId = 'testId'; - note.aliases = [Alias.create('testAlias', true)]; + note.aliases = [Alias.create('testAlias', note, true) as Alias]; note.title = 'testTitle'; note.description = 'testDescription'; note.owner = user; diff --git a/src/notes/utils.spec.ts b/src/notes/utils.spec.ts index 55bc5e302..c84f6cc76 100644 --- a/src/notes/utils.spec.ts +++ b/src/notes/utils.spec.ts @@ -27,10 +27,10 @@ describe('getPrimaryAlias', () => { let note: Note; beforeEach(() => { const user = User.create('hardcoded', 'Testy') as User; - note = Note.create(user, alias); + note = Note.create(user, alias) as Note; }); it('finds correct primary alias', () => { - note.aliases.push(Alias.create('annother', false)); + note.aliases.push(Alias.create('annother', note, false) as Alias); expect(getPrimaryAlias(note)).toEqual(alias); }); it('returns undefined if there is no alias', () => { diff --git a/src/permissions/permissions.service.spec.ts b/src/permissions/permissions.service.spec.ts index 0ff6729ec..d7452b6b3 100644 --- a/src/permissions/permissions.service.spec.ts +++ b/src/permissions/permissions.service.spec.ts @@ -265,30 +265,36 @@ describe('PermissionsService', () => { const everybody: Group = Group.create( SpecialGroup.EVERYONE, SpecialGroup.EVERYONE, - ); + ) as Group; everybody.special = true; result[SpecialGroup.EVERYONE] = everybody; const loggedIn = Group.create( SpecialGroup.LOGGED_IN, SpecialGroup.LOGGED_IN, - ); + ) as Group; loggedIn.special = true; result[SpecialGroup.LOGGED_IN] = loggedIn; - const user1group = Group.create('user1group', 'user1group'); + const user1group = Group.create('user1group', 'user1group') as Group; user1group.members = [user1]; result['user1group'] = user1group; - const user2group = Group.create('user2group', 'user2group'); + const user2group = Group.create('user2group', 'user2group') as Group; user2group.members = [user2]; result['user2group'] = user2group; - const user1and2group = Group.create('user1and2group', 'user1and2group'); + const user1and2group = Group.create( + 'user1and2group', + 'user1and2group', + ) as Group; user1and2group.members = [user1, user2]; result['user1and2group'] = user1and2group; - const user2and1group = Group.create('user2and1group', 'user2and1group'); + const user2and1group = Group.create( + 'user2and1group', + 'user2and1group', + ) as Group; user2and1group.members = [user2, user1]; result['user2and1group'] = user2and1group; @@ -308,7 +314,7 @@ describe('PermissionsService', () => { group: Group, write: boolean, ): NoteGroupPermission { - return NoteGroupPermission.create(group, write); + return NoteGroupPermission.create(group, {} as Note, write); } const everybodyRead = createNoteGroupPermission( diff --git a/src/revisions/revisions.service.spec.ts b/src/revisions/revisions.service.spec.ts index 751721334..aacd68806 100644 --- a/src/revisions/revisions.service.spec.ts +++ b/src/revisions/revisions.service.spec.ts @@ -89,7 +89,8 @@ describe('RevisionsService', () => { describe('getRevision', () => { it('returns a revision', async () => { - const revision = Revision.create('', ''); + const note = {} as Note; + const revision = Revision.create('', '', note) as Revision; jest.spyOn(revisionRepo, 'findOne').mockResolvedValueOnce(revision); expect(await service.getRevision({} as Note, 1)).toEqual(revision); }); @@ -106,11 +107,11 @@ describe('RevisionsService', () => { const note = {} as Note; note.id = 'test'; let revisions: Revision[] = []; - const revision1 = Revision.create('a', 'a'); + const revision1 = Revision.create('a', 'a', note) as Revision; revision1.id = 1; - const revision2 = Revision.create('b', 'b'); + const revision2 = Revision.create('b', 'b', note) as Revision; revision2.id = 2; - const revision3 = Revision.create('c', 'c'); + const revision3 = Revision.create('c', 'c', note) as Revision; revision3.id = 3; revisions.push(revision1, revision2, revision3); note.revisions = Promise.resolve(revisions); @@ -136,7 +137,7 @@ describe('RevisionsService', () => { const note = {} as Note; note.id = 'test'; let revisions: Revision[] = []; - const revision1 = Revision.create('a', 'a'); + const revision1 = Revision.create('a', 'a', note) as Revision; revision1.id = 1; revisions.push(revision1); note.revisions = Promise.resolve(revisions); From d5c77613bb2c8acf43c92d49be9ccbc0be4dcfbb Mon Sep 17 00:00:00 2001 From: Philip Molares Date: Mon, 8 Nov 2021 21:40:36 +0100 Subject: [PATCH 04/12] test(private-api): fix aliases expect Signed-off-by: Philip Molares --- test/private-api/history.e2e-spec.ts | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/test/private-api/history.e2e-spec.ts b/test/private-api/history.e2e-spec.ts index ec10b132e..6db4ab1ef 100644 --- a/test/private-api/history.e2e-spec.ts +++ b/test/private-api/history.e2e-spec.ts @@ -104,7 +104,13 @@ describe('History', () => { .expect(201); const userEntries = await testSetup.historyService.getEntriesByUser(user); expect(userEntries.length).toEqual(1); - expect(userEntries[0].note.aliases).toEqual(note2.aliases); + expect(userEntries[0].note.aliases[0].name).toEqual( + note2.aliases[0].name, + ); + expect(userEntries[0].note.aliases[0].primary).toEqual( + note2.aliases[0].primary, + ); + expect(userEntries[0].note.aliases[0].id).toEqual(note2.aliases[0].id); expect(userEntries[0].user.username).toEqual(user.username); expect(userEntries[0].pinStatus).toEqual(pinStatus); expect(userEntries[0].updatedAt).toEqual(lastVisited); From 09f0d7c3890335640d527ac58b5e5a6b938014d5 Mon Sep 17 00:00:00 2001 From: Philip Molares Date: Thu, 11 Nov 2021 22:06:30 +0100 Subject: [PATCH 05/12] fix(media-upload): rework create and media services saveFile This was done to make the create method more concise. Signed-off-by: Philip Molares --- src/media/media-upload.entity.ts | 25 ++++++++++++++++--------- src/media/media.service.ts | 15 ++++++++------- 2 files changed, 24 insertions(+), 16 deletions(-) diff --git a/src/media/media-upload.entity.ts b/src/media/media-upload.entity.ts index c7259335b..a396fb9f5 100644 --- a/src/media/media-upload.entity.ts +++ b/src/media/media-upload.entity.ts @@ -3,7 +3,6 @@ * * SPDX-License-Identifier: AGPL-3.0-only */ -import * as crypto from 'crypto'; import { Column, CreateDateColumn, @@ -53,24 +52,32 @@ export class MediaUpload { // eslint-disable-next-line @typescript-eslint/no-empty-function private constructor() {} + /** + * Create a new media upload enity + * @param id the id of the upload + * @param note the note the upload should be associated with. This is required despite the fact the note field is optional, because it's possible to delete a note without also deleting the associated media uploads, but a note is required for the initial creation. + * @param user the user that owns the upload + * @param extension which file extension the upload has + * @param backendType on which type of media backend the upload is saved + * @param backendData the backend data returned by the media backend + * @param fileUrl the url where the upload can be accessed + */ public static create( + id: string, note: Note, user: User, extension: string, backendType: BackendType, - backendData?: string, + backendData: BackendData | null, + fileUrl: string, ): Omit { const upload = new MediaUpload(); - const randomBytes = crypto.randomBytes(16); - upload.id = randomBytes.toString('hex') + '.' + extension; + upload.id = id; upload.note = note; upload.user = user; upload.backendType = backendType; - if (backendData) { - upload.backendData = backendData; - } else { - upload.backendData = null; - } + upload.backendData = backendData; + upload.fileUrl = fileUrl; return upload; } } diff --git a/src/media/media.service.ts b/src/media/media.service.ts index 31a7c7b96..20d00c4cc 100644 --- a/src/media/media.service.ts +++ b/src/media/media.service.ts @@ -6,6 +6,7 @@ import { Inject, Injectable } from '@nestjs/common'; import { ModuleRef } from '@nestjs/core'; import { InjectRepository } from '@nestjs/typeorm'; +import crypto from 'crypto'; import * as FileType from 'file-type'; import { Repository } from 'typeorm'; @@ -88,19 +89,19 @@ export class MediaService { if (!MediaService.isAllowedMimeType(fileTypeResult.mime)) { throw new ClientError('MIME type not allowed.'); } + const randomBytes = crypto.randomBytes(16); + const id = randomBytes.toString('hex') + '.' + fileTypeResult.ext; + this.logger.debug(`Generated filename: '${id}'`, 'saveFile'); + const [url, backendData] = await this.mediaBackend.saveFile(fileBuffer, id); const mediaUpload = MediaUpload.create( + id, note, user, fileTypeResult.ext, this.mediaBackendType, + backendData, + url, ); - this.logger.debug(`Generated filename: '${mediaUpload.id}'`, 'saveFile'); - const [url, backendData] = await this.mediaBackend.saveFile( - fileBuffer, - mediaUpload.id, - ); - mediaUpload.backendData = backendData; - mediaUpload.fileUrl = url; await this.mediaUploadRepository.save(mediaUpload); return url; } From 8c78656f8cdfda06ba2ab5428f1dd9fe6f58ba3b Mon Sep 17 00:00:00 2001 From: Philip Molares Date: Thu, 11 Nov 2021 22:07:00 +0100 Subject: [PATCH 06/12] docs: add dev documentation for create methods Signed-off-by: Philip Molares --- docs/content/dev/2.0.md | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/docs/content/dev/2.0.md b/docs/content/dev/2.0.md index 6b92657c7..797dd67ad 100644 --- a/docs/content/dev/2.0.md +++ b/docs/content/dev/2.0.md @@ -28,3 +28,14 @@ The software provides two special groups which have no explicit users: - Run `yarn install` in both projects - Start the backend server in watch mode using `yarn start:dev`. The backend is then accessible on port 3000. - Start the frontend dev server using `yarn start`. The frontend is now accessible on port 3001. + +## Entity `create` methods + +Because we need to have empty constructors in our entity classes for TypeORM to work, the actual constructor is a separate `create` method. These methods should adhere to these guidelines: + +- Only require the non-optional properties of the corresponding entity +- Have no optional parameters +- Have no lists which can be empty (so probably most of them) +- Should either return a complete and fully useable instance or return a Pick/Omit type. +- Exceptions to these rules are allowed, if they are mentioned in the method documentation + From 5ba6b4ab67698de7400b416546404046e6e91ddb Mon Sep 17 00:00:00 2001 From: David Mehren Date: Sun, 14 Nov 2021 20:53:45 +0100 Subject: [PATCH 07/12] fix(group): add special flag to create method To make the create method more consistent with the guidelines, this commit adds the `special` flag to the parameters. As this function will only be used to create the two hard-coded groups and to handle API requests at one or two places, adding the parameter should not be too problematic. Signed-off-by: David Mehren --- src/groups/group.entity.ts | 8 ++++++-- src/groups/groups.service.spec.ts | 2 +- src/groups/groups.service.ts | 3 +-- src/notes/notes.service.spec.ts | 7 ++++--- src/permissions/permissions.service.spec.ts | 10 ++++++---- 5 files changed, 18 insertions(+), 12 deletions(-) diff --git a/src/groups/group.entity.ts b/src/groups/group.entity.ts index 083b4fb9e..f274f90c5 100644 --- a/src/groups/group.entity.ts +++ b/src/groups/group.entity.ts @@ -43,11 +43,15 @@ export class Group { // eslint-disable-next-line @typescript-eslint/no-empty-function private constructor() {} - public static create(name: string, displayName: string): Omit { + public static create( + name: string, + displayName: string, + special: boolean, + ): Omit { const newGroup = new Group(); newGroup.name = name; newGroup.displayName = displayName; - newGroup.special = false; // this attribute should only be true for the two special groups + newGroup.special = special; // this attribute should only be true for the two special groups newGroup.members = []; return newGroup; } diff --git a/src/groups/groups.service.spec.ts b/src/groups/groups.service.spec.ts index 1a776bed2..4b6099bd2 100644 --- a/src/groups/groups.service.spec.ts +++ b/src/groups/groups.service.spec.ts @@ -39,7 +39,7 @@ describe('GroupsService', () => { service = module.get(GroupsService); groupRepo = module.get>(getRepositoryToken(Group)); - group = Group.create('testGroup', 'Superheros') as Group; + group = Group.create('testGroup', 'Superheros', false) as Group; }); it('should be defined', () => { diff --git a/src/groups/groups.service.ts b/src/groups/groups.service.ts index 9e762fd8b..c0e9605e2 100644 --- a/src/groups/groups.service.ts +++ b/src/groups/groups.service.ts @@ -35,8 +35,7 @@ export class GroupsService { displayName: string, special = false, ): Promise { - const group = Group.create(name, displayName); - group.special = special; + const group = Group.create(name, displayName, special); try { return await this.groupRepository.save(group); } catch { diff --git a/src/notes/notes.service.spec.ts b/src/notes/notes.service.spec.ts index 53ed25221..72454fce0 100644 --- a/src/notes/notes.service.spec.ts +++ b/src/notes/notes.service.spec.ts @@ -365,6 +365,7 @@ describe('NotesService', () => { const group = Group.create( groupPermissionUpate.groupname, groupPermissionUpate.groupname, + false, ) as Group; const note = Note.create(user) as Note; describe('works', () => { @@ -668,7 +669,7 @@ describe('NotesService', () => { describe('toNotePermissionsDto', () => { it('works', async () => { const user = User.create('hardcoded', 'Testy') as User; - const group = Group.create('testGroup', 'testGroup') as Group; + const group = Group.create('testGroup', 'testGroup', false) as Group; const note = Note.create(user) as Note; note.userPermissions = [ { @@ -703,7 +704,7 @@ describe('NotesService', () => { const user = User.create('hardcoded', 'Testy') as User; const author = Author.create(1); author.user = user; - const group = Group.create('testGroup', 'testGroup') as Group; + const group = Group.create('testGroup', 'testGroup', false) as Group; const content = 'testContent'; jest .spyOn(noteRepo, 'save') @@ -800,7 +801,7 @@ describe('NotesService', () => { author.user = user; const otherUser = User.create('other hardcoded', 'Testy2') as User; otherUser.username = 'other hardcoded user'; - const group = Group.create('testGroup', 'testGroup') as Group; + const group = Group.create('testGroup', 'testGroup', false) as Group; const content = 'testContent'; jest .spyOn(noteRepo, 'save') diff --git a/src/permissions/permissions.service.spec.ts b/src/permissions/permissions.service.spec.ts index d7452b6b3..981be252e 100644 --- a/src/permissions/permissions.service.spec.ts +++ b/src/permissions/permissions.service.spec.ts @@ -265,28 +265,29 @@ describe('PermissionsService', () => { const everybody: Group = Group.create( SpecialGroup.EVERYONE, SpecialGroup.EVERYONE, + true, ) as Group; - everybody.special = true; result[SpecialGroup.EVERYONE] = everybody; const loggedIn = Group.create( SpecialGroup.LOGGED_IN, SpecialGroup.LOGGED_IN, + true, ) as Group; - loggedIn.special = true; result[SpecialGroup.LOGGED_IN] = loggedIn; - const user1group = Group.create('user1group', 'user1group') as Group; + const user1group = Group.create('user1group', 'user1group', false) as Group; user1group.members = [user1]; result['user1group'] = user1group; - const user2group = Group.create('user2group', 'user2group') as Group; + const user2group = Group.create('user2group', 'user2group', false) as Group; user2group.members = [user2]; result['user2group'] = user2group; const user1and2group = Group.create( 'user1and2group', 'user1and2group', + false, ) as Group; user1and2group.members = [user1, user2]; result['user1and2group'] = user1and2group; @@ -294,6 +295,7 @@ describe('PermissionsService', () => { const user2and1group = Group.create( 'user2and1group', 'user2and1group', + false, ) as Group; user2and1group.members = [user2, user1]; result['user2and1group'] = user2and1group; From a08d8c58ed34568e98728b4ee07c86cda10edba8 Mon Sep 17 00:00:00 2001 From: David Mehren Date: Sun, 14 Nov 2021 20:56:17 +0100 Subject: [PATCH 08/12] fix(identity): remove default for syncSource To make the create method more consistent with the guidelines, this commit removes the default value from the `syncSource` parameter. An Identity will be created as sync source, when the associated account is created using an external provider. Signed-off-by: David Mehren --- src/identity/identity.entity.ts | 2 +- src/identity/identity.service.spec.ts | 6 +++++- src/identity/identity.service.ts | 2 +- 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/src/identity/identity.entity.ts b/src/identity/identity.entity.ts index 512070d3b..73d0964ee 100644 --- a/src/identity/identity.entity.ts +++ b/src/identity/identity.entity.ts @@ -100,7 +100,7 @@ export class Identity { public static create( user: User, providerType: ProviderType, - syncSource = false, + syncSource: boolean, ): Omit { const newIdentity = new Identity(); newIdentity.user = user; diff --git a/src/identity/identity.service.spec.ts b/src/identity/identity.service.spec.ts index 654a0bea3..c7c44ac41 100644 --- a/src/identity/identity.service.spec.ts +++ b/src/identity/identity.service.spec.ts @@ -95,7 +95,11 @@ describe('IdentityService', () => { describe('loginWithLocalIdentity', () => { it('works', async () => { - const identity = Identity.create(user, ProviderType.LOCAL) as Identity; + const identity = Identity.create( + user, + ProviderType.LOCAL, + false, + ) as Identity; identity.passwordHash = await hashPassword(password); user.identities = Promise.resolve([identity]); await expect( diff --git a/src/identity/identity.service.ts b/src/identity/identity.service.ts index 3dc1eb45b..22e573b27 100644 --- a/src/identity/identity.service.ts +++ b/src/identity/identity.service.ts @@ -36,7 +36,7 @@ export class IdentityService { * @return {Identity} the new local identity */ async createLocalIdentity(user: User, password: string): Promise { - const identity = Identity.create(user, ProviderType.LOCAL); + const identity = Identity.create(user, ProviderType.LOCAL, false); identity.passwordHash = await hashPassword(password); return await this.identityRepository.save(identity); } From 9258863dbdb8bc039d7f68fd4e94a23f218b5971 Mon Sep 17 00:00:00 2001 From: David Mehren Date: Sun, 14 Nov 2021 21:14:03 +0100 Subject: [PATCH 09/12] fix(media-upload): remove backendData parameter `Create` methods should only contain optional properties Signed-off-by: David Mehren --- src/media/media-upload.entity.ts | 3 +-- src/media/media.service.ts | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/media/media-upload.entity.ts b/src/media/media-upload.entity.ts index a396fb9f5..7a6d71e13 100644 --- a/src/media/media-upload.entity.ts +++ b/src/media/media-upload.entity.ts @@ -68,7 +68,6 @@ export class MediaUpload { user: User, extension: string, backendType: BackendType, - backendData: BackendData | null, fileUrl: string, ): Omit { const upload = new MediaUpload(); @@ -76,7 +75,7 @@ export class MediaUpload { upload.note = note; upload.user = user; upload.backendType = backendType; - upload.backendData = backendData; + upload.backendData = null; upload.fileUrl = fileUrl; return upload; } diff --git a/src/media/media.service.ts b/src/media/media.service.ts index 20d00c4cc..df8004aca 100644 --- a/src/media/media.service.ts +++ b/src/media/media.service.ts @@ -99,9 +99,9 @@ export class MediaService { user, fileTypeResult.ext, this.mediaBackendType, - backendData, url, ); + mediaUpload.backendData = backendData; await this.mediaUploadRepository.save(mediaUpload); return url; } From 01b53d3858b87fea12472838ebb7c510fb46794d Mon Sep 17 00:00:00 2001 From: David Mehren Date: Sun, 14 Nov 2021 21:22:22 +0100 Subject: [PATCH 10/12] fix(alias): remove default for primary To make the create method more consistent with the guidelines, this commit removes the default value from the `primary` parameter. Signed-off-by: David Mehren --- src/notes/alias.entity.ts | 2 +- src/notes/alias.service.spec.ts | 8 ++++---- src/notes/alias.service.ts | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/notes/alias.entity.ts b/src/notes/alias.entity.ts index 0be2a7d89..70a633f09 100644 --- a/src/notes/alias.entity.ts +++ b/src/notes/alias.entity.ts @@ -54,7 +54,7 @@ export class Alias { // eslint-disable-next-line @typescript-eslint/no-empty-function private constructor() {} - static create(name: string, note: Note, primary = false): Omit { + static create(name: string, note: Note, primary: boolean): Omit { const alias = new Alias(); alias.name = name; alias.primary = primary; diff --git a/src/notes/alias.service.spec.ts b/src/notes/alias.service.spec.ts index 5db8679e3..218b04c30 100644 --- a/src/notes/alias.service.spec.ts +++ b/src/notes/alias.service.spec.ts @@ -140,7 +140,7 @@ describe('AliasService', () => { it('with an already used name', async () => { jest .spyOn(aliasRepo, 'findOne') - .mockResolvedValueOnce(Alias.create(alias2, note) as Alias); + .mockResolvedValueOnce(Alias.create(alias2, note, false) as Alias); await expect(service.addAlias(note, alias2)).rejects.toThrow( AlreadyInDBError, ); @@ -159,7 +159,7 @@ describe('AliasService', () => { const user = User.create('hardcoded', 'Testy') as User; describe('removes one alias correctly', () => { const note = Note.create(user, alias) as Note; - note.aliases.push(Alias.create(alias2, note) as Alias); + note.aliases.push(Alias.create(alias2, note, false) as Alias); it('with two aliases', async () => { jest .spyOn(noteRepo, 'save') @@ -189,7 +189,7 @@ describe('AliasService', () => { }); describe('does not remove one alias', () => { const note = Note.create(user, alias) as Note; - note.aliases.push(Alias.create(alias2, note) as Alias); + note.aliases.push(Alias.create(alias2, note, false) as Alias); it('if the alias is unknown', async () => { await expect(service.removeAlias(note, 'non existent')).rejects.toThrow( NotInDBError, @@ -208,7 +208,7 @@ describe('AliasService', () => { const aliasName = 'testAlias'; const note = Note.create(user, aliasName) as Note; const alias = Alias.create(aliasName, note, true) as Alias; - const alias2 = Alias.create('testAlias2', note) as Alias; + const alias2 = Alias.create('testAlias2', note, false) as Alias; note.aliases.push(alias2); it('mark the alias as primary', async () => { jest diff --git a/src/notes/alias.service.ts b/src/notes/alias.service.ts index 1d5b952b3..96f8c0d55 100644 --- a/src/notes/alias.service.ts +++ b/src/notes/alias.service.ts @@ -66,7 +66,7 @@ export class AliasService { // the first alias is automatically made the primary alias newAlias = Alias.create(alias, note, true); } else { - newAlias = Alias.create(alias, note); + newAlias = Alias.create(alias, note, false); } note.aliases.push(newAlias as Alias); From 9c08ff94fe8ea2589e48a8dcd528b2af3d1a5ec1 Mon Sep 17 00:00:00 2001 From: David Mehren Date: Sun, 14 Nov 2021 21:44:59 +0100 Subject: [PATCH 11/12] 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 --- src/api/private/notes/notes.controller.ts | 4 ++-- src/api/public/notes/notes.controller.ts | 4 ++-- src/notes/note.entity.ts | 9 +++++-- src/notes/notes.service.spec.ts | 22 ++++++++--------- src/notes/notes.service.ts | 2 +- test/private-api/alias.e2e-spec.ts | 6 ++--- test/private-api/history.e2e-spec.ts | 4 ++-- test/private-api/me.e2e-spec.ts | 4 ++-- test/private-api/media.e2e-spec.ts | 2 ++ test/private-api/notes.e2e-spec.ts | 18 +++++++------- test/public-api/alias.e2e-spec.ts | 6 ++--- test/public-api/me.e2e-spec.ts | 16 ++++++------- test/public-api/media.e2e-spec.ts | 1 + test/public-api/notes.e2e-spec.ts | 29 +++++++++++++---------- 14 files changed, 69 insertions(+), 58 deletions(-) diff --git a/src/api/private/notes/notes.controller.ts b/src/api/private/notes/notes.controller.ts index 9397456b3..0b829ebb6 100644 --- a/src/api/private/notes/notes.controller.ts +++ b/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) { diff --git a/src/api/public/notes/notes.controller.ts b/src/api/public/notes/notes.controller.ts index 81c719b5d..31172cc78 100644 --- a/src/api/public/notes/notes.controller.ts +++ b/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) { diff --git a/src/notes/note.entity.ts b/src/notes/note.entity.ts index 505db302f..42e697672 100644 --- a/src/notes/note.entity.ts +++ b/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 { + /** + * 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 { 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 = []; diff --git a/src/notes/notes.service.spec.ts b/src/notes/notes.service.spec.ts index 72454fce0..3be7f998f 100644 --- a/src/notes/notes.service.spec.ts +++ b/src/notes/notes.service.spec.ts @@ -164,7 +164,7 @@ describe('NotesService', () => { .mockImplementation(async (note: Note): Promise => 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); - 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); - 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); - 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); - 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); - const note = await service.createNote(content); + const note = await service.createNote(content, null); const revisions = await note.revisions; revisions[0].edits = [ { diff --git a/src/notes/notes.service.ts b/src/notes/notes.service.ts index 5ea656cd2..87f799ed2 100644 --- a/src/notes/notes.service.ts +++ b/src/notes/notes.service.ts @@ -88,8 +88,8 @@ export class NotesService { */ async createNote( noteContent: string, + owner: User | null, alias?: string, - owner?: User, ): Promise { if (alias) { this.checkNoteIdOrAlias(alias); diff --git a/test/private-api/alias.e2e-spec.ts b/test/private-api/alias.e2e-spec.ts index 00056543b..c62ac1481 100644 --- a/test/private-api/alias.e2e-spec.ts +++ b/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); }); diff --git a/test/private-api/history.e2e-spec.ts b/test/private-api/history.e2e-spec.ts index 6db4ab1ef..de07bee27 100644 --- a/test/private-api/history.e2e-spec.ts +++ b/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); - note2 = await notesService.createNote(content, 'note2', user); + note = await notesService.createNote(content, user, 'note'); + note2 = await notesService.createNote(content, user, 'note2'); agent = request.agent(testSetup.app.getHttpServer()); await agent .post('/api/private/auth/local/login') diff --git a/test/private-api/me.e2e-spec.ts b/test/private-api/me.e2e-spec.ts index 54e80ab7b..97071d73d 100644 --- a/test/private-api/me.e2e-spec.ts +++ b/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); - note2 = await testSetup.notesService.createNote(content, alias2, user); + note1 = await testSetup.notesService.createNote(content, user); + note2 = await testSetup.notesService.createNote(content, user, alias2); agent = request.agent(testSetup.app.getHttpServer()); await agent .post('/api/private/auth/local/login') diff --git a/test/private-api/media.e2e-spec.ts b/test/private-api/media.e2e-spec.ts index 2160ce71f..448b1ecef 100644 --- a/test/private-api/media.e2e-spec.ts +++ b/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'); diff --git a/test/private-api/notes.e2e-spec.ts b/test/private-api/notes.e2e-spec.ts index 1589c6d3d..5e800e89d 100644 --- a/test/private-api/notes.e2e-spec.ts +++ b/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/`) diff --git a/test/public-api/alias.e2e-spec.ts b/test/public-api/alias.e2e-spec.ts index c8c8284b9..149808e8d 100644 --- a/test/public-api/alias.e2e-spec.ts +++ b/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); }); diff --git a/test/public-api/me.e2e-spec.ts b/test/public-api/me.e2e-spec.ts index a52a52137..2e02fc6aa 100644 --- a/test/public-api/me.e2e-spec.ts +++ b/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) diff --git a/test/public-api/media.e2e-spec.ts b/test/public-api/media.e2e-spec.ts index c2ad6e4de..913b498df 100644 --- a/test/public-api/media.e2e-spec.ts +++ b/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', ); }); diff --git a/test/public-api/notes.e2e-spec.ts b/test/public-api/notes.e2e-spec.ts index 487c8a58e..971af9913 100644 --- a/test/public-api/notes.e2e-spec.ts +++ b/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/`) From db1d44cb69e92eaef1fcacbdcaed14083cd8d43a Mon Sep 17 00:00:00 2001 From: David Mehren Date: Sun, 14 Nov 2021 21:49:31 +0100 Subject: [PATCH 12/12] fix(seed): fix create method usage Signed-off-by: David Mehren --- src/seed.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/seed.ts b/src/seed.ts index b66300909..0f89ce1f5 100644 --- a/src/seed.ts +++ b/src/seed.ts @@ -57,14 +57,14 @@ createConnection({ users.push(User.create('hardcoded_2', 'Test User 2')); users.push(User.create('hardcoded_3', 'Test User 3')); const notes: Note[] = []; - notes.push(Note.create(undefined, 'test') as Note); - notes.push(Note.create(undefined, 'test2') as Note); - notes.push(Note.create(undefined, 'test3') as Note); + notes.push(Note.create(null, 'test') as Note); + notes.push(Note.create(null, 'test2') as Note); + notes.push(Note.create(null, 'test3') as Note); for (let i = 0; i < 3; i++) { const author = connection.manager.create(Author, Author.create(1)); const user = connection.manager.create(User, users[i]); - const identity = Identity.create(user, ProviderType.LOCAL); + const identity = Identity.create(user, ProviderType.LOCAL, false); identity.passwordHash = await hashPassword(password); connection.manager.create(Identity, identity); author.user = user;