From df44b32ec2c79ea2afdec7442f461b1b163322ae Mon Sep 17 00:00:00 2001 From: dependentmadani Date: Thu, 29 Aug 2024 11:26:26 +0100 Subject: [PATCH 01/31] feat(parent structure): add method that return note parent structure --- src/domain/index.ts | 2 +- src/domain/service/note.ts | 46 +++++++++++++++++++++++++++++++++++++- 2 files changed, 46 insertions(+), 2 deletions(-) diff --git a/src/domain/index.ts b/src/domain/index.ts index 44ed849b..7a0abf0b 100644 --- a/src/domain/index.ts +++ b/src/domain/index.ts @@ -63,7 +63,7 @@ export function init(repositories: Repositories, appConfig: AppConfig): DomainSe /** * @todo use shared methods for uncoupling repositories unrelated to note service */ - const noteService = new NoteService(repositories.noteRepository, repositories.noteRelationsRepository, repositories.noteVisitsRepository, repositories.editorToolsRepository, repositories.noteHistoryRepository); + const noteService = new NoteService(repositories.noteRepository, repositories.noteRelationsRepository, repositories.noteVisitsRepository, repositories.editorToolsRepository, repositories.noteHistoryRepository, repositories.teamRepository); const noteVisitsService = new NoteVisitsService(repositories.noteVisitsRepository); const authService = new AuthService( appConfig.auth.accessSecret, diff --git a/src/domain/service/note.ts b/src/domain/service/note.ts index d6272ae9..387c36d9 100644 --- a/src/domain/service/note.ts +++ b/src/domain/service/note.ts @@ -9,6 +9,7 @@ import type User from '@domain/entities/user.js'; import type { NoteList } from '@domain/entities/noteList.js'; import type NoteHistoryRepository from '@repository/noteHistory.repository.js'; import type { NoteHistoryMeta, NoteHistoryRecord, NoteHistoryPublic } from '@domain/entities/noteHistory.js'; +import type TeamRepository from '@repository/team.repository.js'; /** * Note service @@ -39,6 +40,11 @@ export default class NoteService { */ public noteHistoryRepository: NoteHistoryRepository; + /** + * Team repository + */ + public teamRepository: TeamRepository; + /** * Number of the notes to be displayed on one page * it is used to calculate offset and limit for getting notes that the user has recently opened @@ -57,13 +63,15 @@ export default class NoteService { * @param noteVisitsRepository - note visits repository * @param editorToolsRepository - editor tools repositoryn * @param noteHistoryRepository - note history repository + * @param teamRepository - team note repository */ - constructor(noteRepository: NoteRepository, noteRelationsRepository: NoteRelationsRepository, noteVisitsRepository: NoteVisitsRepository, editorToolsRepository: EditorToolsRepository, noteHistoryRepository: NoteHistoryRepository) { + constructor(noteRepository: NoteRepository, noteRelationsRepository: NoteRelationsRepository, noteVisitsRepository: NoteVisitsRepository, editorToolsRepository: EditorToolsRepository, noteHistoryRepository: NoteHistoryRepository, teamRepository: TeamRepository) { this.noteRepository = noteRepository; this.noteRelationsRepository = noteRelationsRepository; this.noteVisitsRepository = noteVisitsRepository; this.editorToolsRepository = editorToolsRepository; this.noteHistoryRepository = noteHistoryRepository; + this.teamRepository = teamRepository; } /** @@ -441,4 +449,40 @@ export default class NoteService { return noteHistoryPublic; } + + /** + * Get note parent structure recursively by note id and user id + * and check if user has access to the parent note. + * @param noteId - id of the note to get parent structure + * @param userId - id of the user that is requesting the parent structure + * @returns - array of notes that are parent structure of the note + */ + public async getNoteParentStructure(noteId: NoteInternalId, userId: number | null): Promise> { + const noteUserAccess = await this.teamRepository.getTeamMemberByNoteAndUserId(userId as NoteInternalId, noteId); + + if (userId === null || noteUserAccess === null) { + const nextNoteId = await this.getParentNoteIdByNoteId(noteId); + + if (nextNoteId === null || userId === null) { + return []; + } + + return [...await this.getNoteParentStructure(nextNoteId, userId)]; + } + + const parentNoteId = await this.getParentNoteIdByNoteId(noteId); + + const noteData = await this.getNoteById(noteId); + + const noteObjectInfo = { + noteId: noteData.publicId, + content: noteData.content, + }; + + if (parentNoteId === null) { + return [noteObjectInfo]; + } + + return [noteObjectInfo, ...await this.getNoteParentStructure(parentNoteId, userId)].reverse(); + } } From 0551b9f714b0cdecfd8e512c94fbd69fef1a2c3a Mon Sep 17 00:00:00 2001 From: dependentmadani Date: Sun, 1 Sep 2024 10:00:46 +0100 Subject: [PATCH 02/31] update: start doing the search in team storage --- src/domain/entities/note.ts | 15 +++++ src/domain/service/note.ts | 31 +-------- src/repository/index.ts | 1 + .../storage/postgres/orm/sequelize/teams.ts | 67 ++++++++++++++++++- src/repository/team.repository.ts | 12 +++- 5 files changed, 96 insertions(+), 30 deletions(-) diff --git a/src/domain/entities/note.ts b/src/domain/entities/note.ts index b393a680..8384b706 100644 --- a/src/domain/entities/note.ts +++ b/src/domain/entities/note.ts @@ -82,3 +82,18 @@ export interface Note { * Part of note entity used to create new note */ export type NoteCreationAttributes = Pick; + +/** + * The return type for note parent structure + */ +export type NoteParentStructure = { + /** + * Note public id + */ + noteId: Note['publicId']; + + /** + * Note content + */ + content: Note['content']; +}; diff --git a/src/domain/service/note.ts b/src/domain/service/note.ts index 387c36d9..e742101b 100644 --- a/src/domain/service/note.ts +++ b/src/domain/service/note.ts @@ -1,4 +1,4 @@ -import type { Note, NoteInternalId, NotePublicId } from '@domain/entities/note.js'; +import type { Note, NoteInternalId, NoteParentStructure, NotePublicId } from '@domain/entities/note.js'; import type NoteRepository from '@repository/note.repository.js'; import type NoteVisitsRepository from '@repository/noteVisits.repository.js'; import { createPublicId } from '@infrastructure/utils/id.js'; @@ -457,32 +457,7 @@ export default class NoteService { * @param userId - id of the user that is requesting the parent structure * @returns - array of notes that are parent structure of the note */ - public async getNoteParentStructure(noteId: NoteInternalId, userId: number | null): Promise> { - const noteUserAccess = await this.teamRepository.getTeamMemberByNoteAndUserId(userId as NoteInternalId, noteId); - - if (userId === null || noteUserAccess === null) { - const nextNoteId = await this.getParentNoteIdByNoteId(noteId); - - if (nextNoteId === null || userId === null) { - return []; - } - - return [...await this.getNoteParentStructure(nextNoteId, userId)]; - } - - const parentNoteId = await this.getParentNoteIdByNoteId(noteId); - - const noteData = await this.getNoteById(noteId); - - const noteObjectInfo = { - noteId: noteData.publicId, - content: noteData.content, - }; - - if (parentNoteId === null) { - return [noteObjectInfo]; - } - - return [noteObjectInfo, ...await this.getNoteParentStructure(parentNoteId, userId)].reverse(); + public async getNoteParentStructure(noteId: NoteInternalId, userId: number): Promise> { + return await this.teamRepository.getAllNotesParents(noteId, userId); } } diff --git a/src/repository/index.ts b/src/repository/index.ts index ed40d01e..d07f5aa1 100644 --- a/src/repository/index.ts +++ b/src/repository/index.ts @@ -135,6 +135,7 @@ export async function init(orm: Orm, s3Config: S3StorageConfig): Promise): void { + this.noteRelationModel = model; + } + /** * Create new team member membership * @param data - team membership data @@ -203,6 +217,57 @@ export default class TeamsSequelizeStorage { }); } + /** + * Get note parent structure + * @param noteId : the ID of the note. + * @param userId : The ID of the user. + */ + public async getAllNoteParents(noteId: NoteInternalId, userId: number): Promise { + if (!this.noteModel || !this.noteRelationModel) { + throw new DomainError('Note model or Note relation model not initialized'); + } + + const parentNotes: NoteParentStructure[] = []; + let currentNoteId: NoteInternalId | null = noteId; + + while (currentNoteId != null) { + // Check if the user has access to the current note + const teamMember = await this.model.findOne({ + where: { + noteId: currentNoteId, + userId, + }, + }); + + if (teamMember) { + // If the user does not have access, add the note to the array + const note = await this.noteModel.findOne({ + where: { id: currentNoteId }, + }); + + if (note) { + parentNotes.push({ + noteId: note.publicId, + content: note.content, + }); + } + } + + // Retrieve the parent note + const noteRelation: NoteRelationsModel | null = await this.noteRelationModel.findOne({ + where: { noteId: currentNoteId }, + }); + + if (noteRelation != null) { + currentNoteId = noteRelation.parentId; + } else { + currentNoteId = null; + } + } + + return parentNotes.reverse(); + } + /** * Remove team member by id * @param userId - id of team member diff --git a/src/repository/team.repository.ts b/src/repository/team.repository.ts index fb01f0d3..0a358fa2 100644 --- a/src/repository/team.repository.ts +++ b/src/repository/team.repository.ts @@ -1,6 +1,6 @@ import type TeamStorage from '@repository/storage/team.storage.js'; import type { Team, TeamMember, TeamMemberCreationAttributes, MemberRole } from '@domain/entities/team.js'; -import type { NoteInternalId } from '@domain/entities/note.js'; +import type { NoteInternalId, NoteParentStructure } from '@domain/entities/note.js'; import type User from '@domain/entities/user.js'; /** @@ -57,6 +57,16 @@ export default class TeamRepository { return await this.storage.getTeamMembersWithUserInfoByNoteId(noteId); }; + /** + * Get all notes parents based on note id and user id, by checking team access + * @param noteId : note id to get all its parents + * @param userId : user id to check access + * @returns an array of note parents objects containing public id and content + */ + public async getAllNotesParents(noteId: NoteInternalId, userId: number): Promise { + return await this.storage.getAllNoteParents(noteId, userId); + } + /** * Remove team member by id * @param userId - id of the team member From a5adcc2af5ab8825b3d211501112db6b5b2b31cc Mon Sep 17 00:00:00 2001 From: dependentmadani Date: Wed, 4 Sep 2024 01:16:58 +0100 Subject: [PATCH 03/31] update: first part of code review is done --- src/domain/entities/note.ts | 4 +- src/domain/service/note.ts | 4 +- src/presentation/http/router/note.ts | 4 ++ .../storage/postgres/orm/sequelize/note.ts | 7 ++-- .../storage/postgres/orm/sequelize/teams.ts | 40 ++++++++++--------- src/repository/team.repository.ts | 4 +- 6 files changed, 34 insertions(+), 29 deletions(-) diff --git a/src/domain/entities/note.ts b/src/domain/entities/note.ts index 8384b706..fbae28a2 100644 --- a/src/domain/entities/note.ts +++ b/src/domain/entities/note.ts @@ -84,9 +84,9 @@ export interface Note { export type NoteCreationAttributes = Pick; /** - * The return type for note parent structure + * Represents the content of a parent note. */ -export type NoteParentStructure = { +export type NoteParentContent = { /** * Note public id */ diff --git a/src/domain/service/note.ts b/src/domain/service/note.ts index e742101b..e1c45319 100644 --- a/src/domain/service/note.ts +++ b/src/domain/service/note.ts @@ -1,4 +1,4 @@ -import type { Note, NoteInternalId, NoteParentStructure, NotePublicId } from '@domain/entities/note.js'; +import type { Note, NoteInternalId, NoteParentContent, NotePublicId } from '@domain/entities/note.js'; import type NoteRepository from '@repository/note.repository.js'; import type NoteVisitsRepository from '@repository/noteVisits.repository.js'; import { createPublicId } from '@infrastructure/utils/id.js'; @@ -457,7 +457,7 @@ export default class NoteService { * @param userId - id of the user that is requesting the parent structure * @returns - array of notes that are parent structure of the note */ - public async getNoteParentStructure(noteId: NoteInternalId, userId: number): Promise> { + public async getNoteParentStructure(noteId: NoteInternalId, userId: number): Promise> { return await this.teamRepository.getAllNotesParents(noteId, userId); } } diff --git a/src/presentation/http/router/note.ts b/src/presentation/http/router/note.ts index 6de6b09d..cc7e9d9b 100644 --- a/src/presentation/http/router/note.ts +++ b/src/presentation/http/router/note.ts @@ -172,6 +172,10 @@ const NoteRouter: FastifyPluginCallback = (fastify, opts, don */ const canEdit = memberRole === MemberRole.Write; + const noteParentContent = await noteService.getNoteParentStructure(noteId, userId!); + + console.log('noteParentContent', noteParentContent); + return reply.send({ note: notePublic, parentNote: parentNote, diff --git a/src/repository/storage/postgres/orm/sequelize/note.ts b/src/repository/storage/postgres/orm/sequelize/note.ts index 4d97591b..d6a4d1ac 100644 --- a/src/repository/storage/postgres/orm/sequelize/note.ts +++ b/src/repository/storage/postgres/orm/sequelize/note.ts @@ -5,7 +5,6 @@ import type { Note, NoteCreationAttributes, NoteInternalId, NotePublicId } from import { UserModel } from '@repository/storage/postgres/orm/sequelize/user.js'; import type { NoteSettingsModel } from './noteSettings.js'; import type { NoteVisitsModel } from './noteVisits.js'; -import { DomainError } from '@domain/entities/DomainError.js'; import type { NoteHistoryModel } from './noteHistory.js'; /* eslint-disable @typescript-eslint/naming-convention */ @@ -233,11 +232,11 @@ export default class NoteSequelizeStorage { */ public async getNoteListByUserId(userId: number, offset: number, limit: number): Promise { if (this.visitsModel === null) { - throw new DomainError('NoteVisit model should be defined'); + throw new Error('NoteStorage: NoteVisit model should be defined'); } if (!this.settingsModel) { - throw new Error('Note settings model not initialized'); + throw new Error('NoteStorage: Note settings model not initialized'); } const reply = await this.model.findAll({ @@ -293,7 +292,7 @@ export default class NoteSequelizeStorage { */ public async getNoteByHostname(hostname: string): Promise { if (!this.settingsModel) { - throw new Error('Note settings model not initialized'); + throw new Error('NoteStorage: Note settings model not initialized'); } /** diff --git a/src/repository/storage/postgres/orm/sequelize/teams.ts b/src/repository/storage/postgres/orm/sequelize/teams.ts index 3e5bfcf4..687e797d 100644 --- a/src/repository/storage/postgres/orm/sequelize/teams.ts +++ b/src/repository/storage/postgres/orm/sequelize/teams.ts @@ -6,8 +6,7 @@ import type { Team, TeamMemberCreationAttributes, TeamMember } from '@domain/ent import { UserModel } from './user.js'; import { MemberRole } from '@domain/entities/team.js'; import type User from '@domain/entities/user.js'; -import type { NoteInternalId, NoteParentStructure } from '@domain/entities/note.js'; -import { DomainError } from '@domain/entities/DomainError.js'; +import type { NoteInternalId, NoteParentContent } from '@domain/entities/note.js'; import type { NoteRelationsModel } from './noteRelations.js'; /** @@ -33,6 +32,11 @@ export class TeamsModel extends Model, InferCreation * Team member role, show what user can do with note */ public declare role: MemberRole; + + /** + * Note relation content + */ + public declare notes?: NoteModel | null; } /** @@ -202,7 +206,7 @@ export default class TeamsSequelizeStorage { */ public async getTeamMembersWithUserInfoByNoteId(noteId: NoteInternalId): Promise { if (!this.userModel) { - throw new DomainError('User model not initialized'); + throw new Error('TeamStorage: User model not defined'); } return await this.model.findAll({ @@ -219,15 +223,15 @@ export default class TeamsSequelizeStorage { /** * Get note parent structure - * @param noteId : the ID of the note. - * @param userId : The ID of the user. + * @param noteId - the ID of the note. + * @param userId - the ID of the user. */ - public async getAllNoteParents(noteId: NoteInternalId, userId: number): Promise { + public async getAllNoteParents(noteId: NoteInternalId, userId: number): Promise { if (!this.noteModel || !this.noteRelationModel) { - throw new DomainError('Note model or Note relation model not initialized'); + throw new Error(`${this.noteModel !== null ? 'TeamStorage: Note relation model is not defined' : 'TeamStorage: Note model is not defined'}`); } - const parentNotes: NoteParentStructure[] = []; + const parentNotes: NoteParentContent[] = []; let currentNoteId: NoteInternalId | null = noteId; while (currentNoteId != null) { @@ -237,20 +241,18 @@ export default class TeamsSequelizeStorage { noteId: currentNoteId, userId, }, + include: { + model: this.noteModel, + as: this.noteModel.tableName, + required: true, + }, }); - if (teamMember) { - // If the user does not have access, add the note to the array - const note = await this.noteModel.findOne({ - where: { id: currentNoteId }, + if (teamMember && teamMember.notes !== undefined && teamMember.notes !== null) { + parentNotes.push({ + noteId: teamMember.notes.publicId, + content: teamMember.notes.content, }); - - if (note) { - parentNotes.push({ - noteId: note.publicId, - content: note.content, - }); - } } // Retrieve the parent note diff --git a/src/repository/team.repository.ts b/src/repository/team.repository.ts index 0a358fa2..4d7e7809 100644 --- a/src/repository/team.repository.ts +++ b/src/repository/team.repository.ts @@ -1,6 +1,6 @@ import type TeamStorage from '@repository/storage/team.storage.js'; import type { Team, TeamMember, TeamMemberCreationAttributes, MemberRole } from '@domain/entities/team.js'; -import type { NoteInternalId, NoteParentStructure } from '@domain/entities/note.js'; +import type { NoteInternalId, NoteParentContent } from '@domain/entities/note.js'; import type User from '@domain/entities/user.js'; /** @@ -63,7 +63,7 @@ export default class TeamRepository { * @param userId : user id to check access * @returns an array of note parents objects containing public id and content */ - public async getAllNotesParents(noteId: NoteInternalId, userId: number): Promise { + public async getAllNotesParents(noteId: NoteInternalId, userId: number): Promise { return await this.storage.getAllNoteParents(noteId, userId); } From bccf7b119ae02a86d2da720c4c23887e1cea965b Mon Sep 17 00:00:00 2001 From: dependentmadani Date: Wed, 4 Sep 2024 11:36:40 +0100 Subject: [PATCH 04/31] fix: finish the review modifications --- src/presentation/http/router/note.ts | 4 ---- .../storage/postgres/orm/sequelize/teams.ts | 21 ++++++++++++++++++- 2 files changed, 20 insertions(+), 5 deletions(-) diff --git a/src/presentation/http/router/note.ts b/src/presentation/http/router/note.ts index cc7e9d9b..6de6b09d 100644 --- a/src/presentation/http/router/note.ts +++ b/src/presentation/http/router/note.ts @@ -172,10 +172,6 @@ const NoteRouter: FastifyPluginCallback = (fastify, opts, don */ const canEdit = memberRole === MemberRole.Write; - const noteParentContent = await noteService.getNoteParentStructure(noteId, userId!); - - console.log('noteParentContent', noteParentContent); - return reply.send({ note: notePublic, parentNote: parentNote, diff --git a/src/repository/storage/postgres/orm/sequelize/teams.ts b/src/repository/storage/postgres/orm/sequelize/teams.ts index 687e797d..35cd005e 100644 --- a/src/repository/storage/postgres/orm/sequelize/teams.ts +++ b/src/repository/storage/postgres/orm/sequelize/teams.ts @@ -233,9 +233,12 @@ export default class TeamsSequelizeStorage { const parentNotes: NoteParentContent[] = []; let currentNoteId: NoteInternalId | null = noteId; + /** + * Store notes that user can not access, to check the inherited team if has access + */ + let storeUnaccessibleNote: NoteParentContent[] = []; while (currentNoteId != null) { - // Check if the user has access to the current note const teamMember = await this.model.findOne({ where: { noteId: currentNoteId, @@ -249,10 +252,26 @@ export default class TeamsSequelizeStorage { }); if (teamMember && teamMember.notes !== undefined && teamMember.notes !== null) { + if (storeUnaccessibleNote.length > 0) { + parentNotes.push(...storeUnaccessibleNote); + storeUnaccessibleNote = []; + } parentNotes.push({ noteId: teamMember.notes.publicId, content: teamMember.notes.content, }); + } else { + const note = await this.noteModel.findOne({ + where: { id: currentNoteId }, + attributes: ['publicId', 'content'], + }); + + if (note !== null) { + storeUnaccessibleNote.push({ + noteId: note.publicId, + content: note.content, + }); + } } // Retrieve the parent note From 42cb421410114668acaa6670c1f21a3d9cab81b4 Mon Sep 17 00:00:00 2001 From: dependentmadani Date: Fri, 6 Sep 2024 18:37:35 +0100 Subject: [PATCH 05/31] update: first part of modification after review --- docker-compose.yml | 2 +- src/domain/entities/note.ts | 11 +++++++++-- .../storage/postgres/orm/sequelize/teams.ts | 14 ++++++++------ src/repository/team.repository.ts | 4 ++-- 4 files changed, 20 insertions(+), 11 deletions(-) diff --git a/docker-compose.yml b/docker-compose.yml index 3573148d..6c9aacf3 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -1,4 +1,3 @@ -version: "3.2" services: api: build: @@ -8,6 +7,7 @@ services: - 127.0.0.1:1337:1337 volumes: - ./app-config.yaml:/usr/app/app-config.yaml + - ./app-config.local.yaml:/usr/app/app-config.local.yaml restart: unless-stopped postgres: diff --git a/src/domain/entities/note.ts b/src/domain/entities/note.ts index fbae28a2..0537c851 100644 --- a/src/domain/entities/note.ts +++ b/src/domain/entities/note.ts @@ -84,9 +84,11 @@ export interface Note { export type NoteCreationAttributes = Pick; /** - * Represents the content of a parent note. + * This type represents only note's content + * Used for work with just note's content in web part + * This type would be used as a part of the structure, that represents all parent notes of the current note */ -export type NoteParentContent = { +export type NoteIdAndContent = { /** * Note public id */ @@ -97,3 +99,8 @@ export type NoteParentContent = { */ content: Note['content']; }; + +/** + * This type represents the structure, that represents all parent notes of the current note + */ +export type NoteParentsStructure = NoteIdAndContent[]; diff --git a/src/repository/storage/postgres/orm/sequelize/teams.ts b/src/repository/storage/postgres/orm/sequelize/teams.ts index 35cd005e..bce9855f 100644 --- a/src/repository/storage/postgres/orm/sequelize/teams.ts +++ b/src/repository/storage/postgres/orm/sequelize/teams.ts @@ -6,8 +6,9 @@ import type { Team, TeamMemberCreationAttributes, TeamMember } from '@domain/ent import { UserModel } from './user.js'; import { MemberRole } from '@domain/entities/team.js'; import type User from '@domain/entities/user.js'; -import type { NoteInternalId, NoteParentContent } from '@domain/entities/note.js'; +import type { NoteInternalId, NoteParentsStructure } from '@domain/entities/note.js'; import type { NoteRelationsModel } from './noteRelations.js'; +import { isEmpty } from '@infrastructure/utils/empty.js'; /** * Class representing a teams model in database @@ -226,17 +227,17 @@ export default class TeamsSequelizeStorage { * @param noteId - the ID of the note. * @param userId - the ID of the user. */ - public async getAllNoteParents(noteId: NoteInternalId, userId: number): Promise { + public async getAllNoteParents(noteId: NoteInternalId, userId: number): Promise { if (!this.noteModel || !this.noteRelationModel) { throw new Error(`${this.noteModel !== null ? 'TeamStorage: Note relation model is not defined' : 'TeamStorage: Note model is not defined'}`); } - const parentNotes: NoteParentContent[] = []; + const parentNotes: NoteParentsStructure = []; let currentNoteId: NoteInternalId | null = noteId; /** * Store notes that user can not access, to check the inherited team if has access */ - let storeUnaccessibleNote: NoteParentContent[] = []; + let storeUnaccessibleNote: NoteParentsStructure = []; while (currentNoteId != null) { const teamMember = await this.model.findOne({ @@ -248,10 +249,11 @@ export default class TeamsSequelizeStorage { model: this.noteModel, as: this.noteModel.tableName, required: true, + attributes: ['publicId', 'content'], }, }); - if (teamMember && teamMember.notes !== undefined && teamMember.notes !== null) { + if (teamMember && !isEmpty(teamMember.notes)) { if (storeUnaccessibleNote.length > 0) { parentNotes.push(...storeUnaccessibleNote); storeUnaccessibleNote = []; @@ -260,7 +262,7 @@ export default class TeamsSequelizeStorage { noteId: teamMember.notes.publicId, content: teamMember.notes.content, }); - } else { + } else if (teamMember === null) { const note = await this.noteModel.findOne({ where: { id: currentNoteId }, attributes: ['publicId', 'content'], diff --git a/src/repository/team.repository.ts b/src/repository/team.repository.ts index 4d7e7809..c426c35f 100644 --- a/src/repository/team.repository.ts +++ b/src/repository/team.repository.ts @@ -1,6 +1,6 @@ import type TeamStorage from '@repository/storage/team.storage.js'; import type { Team, TeamMember, TeamMemberCreationAttributes, MemberRole } from '@domain/entities/team.js'; -import type { NoteInternalId, NoteParentContent } from '@domain/entities/note.js'; +import type { NoteInternalId, NoteParentsStructure } from '@domain/entities/note.js'; import type User from '@domain/entities/user.js'; /** @@ -63,7 +63,7 @@ export default class TeamRepository { * @param userId : user id to check access * @returns an array of note parents objects containing public id and content */ - public async getAllNotesParents(noteId: NoteInternalId, userId: number): Promise { + public async getAllNotesParents(noteId: NoteInternalId, userId: number): Promise { return await this.storage.getAllNoteParents(noteId, userId); } From bb6caff6c5b6e0faae73fd358b1af5fa6420acf1 Mon Sep 17 00:00:00 2001 From: dependentmadani Date: Fri, 6 Sep 2024 18:51:37 +0100 Subject: [PATCH 06/31] fix: build problem fixed --- src/domain/service/note.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/domain/service/note.ts b/src/domain/service/note.ts index e1c45319..fd073182 100644 --- a/src/domain/service/note.ts +++ b/src/domain/service/note.ts @@ -1,4 +1,4 @@ -import type { Note, NoteInternalId, NoteParentContent, NotePublicId } from '@domain/entities/note.js'; +import type { Note, NoteInternalId, NoteParentsStructure, NotePublicId } from '@domain/entities/note.js'; import type NoteRepository from '@repository/note.repository.js'; import type NoteVisitsRepository from '@repository/noteVisits.repository.js'; import { createPublicId } from '@infrastructure/utils/id.js'; @@ -457,7 +457,7 @@ export default class NoteService { * @param userId - id of the user that is requesting the parent structure * @returns - array of notes that are parent structure of the note */ - public async getNoteParentStructure(noteId: NoteInternalId, userId: number): Promise> { + public async getNoteParentStructure(noteId: NoteInternalId, userId: number): Promise { return await this.teamRepository.getAllNotesParents(noteId, userId); } } From a11d1f8e107dd7f21d1c6e71509b11a89f7e4142 Mon Sep 17 00:00:00 2001 From: dependentmadani Date: Sat, 7 Sep 2024 19:45:03 +0100 Subject: [PATCH 07/31] update(note request): update the content send in get request of note by adding note parrent structure --- src/presentation/http/router/note.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/presentation/http/router/note.ts b/src/presentation/http/router/note.ts index 6de6b09d..31ca6824 100644 --- a/src/presentation/http/router/note.ts +++ b/src/presentation/http/router/note.ts @@ -2,7 +2,7 @@ import type { FastifyPluginCallback } from 'fastify'; import type NoteService from '@domain/service/note.js'; import type NoteSettingsService from '@domain/service/noteSettings.js'; import type { ErrorResponse } from '@presentation/http/types/HttpResponse.js'; -import type { Note, NotePublicId } from '@domain/entities/note.js'; +import type { Note, NoteParentsStructure, NotePublicId } from '@domain/entities/note.js'; import useNoteResolver from '../middlewares/note/useNoteResolver.js'; import useNoteSettingsResolver from '../middlewares/noteSettings/useNoteSettingsResolver.js'; import useMemberRoleResolver from '../middlewares/noteSettings/useMemberRoleResolver.js'; @@ -85,6 +85,7 @@ const NoteRouter: FastifyPluginCallback = (fastify, opts, don canEdit: boolean; }; tools: EditorTool[]; + parentStructure: NoteParentsStructure; } | ErrorResponse; }>('/:notePublicId', { config: { @@ -172,11 +173,14 @@ const NoteRouter: FastifyPluginCallback = (fastify, opts, don */ const canEdit = memberRole === MemberRole.Write; + const noteParentStructure = await noteService.getNoteParentStructure(noteId, userId!); + return reply.send({ note: notePublic, parentNote: parentNote, accessRights: { canEdit: canEdit }, tools: noteTools, + parentStructure: noteParentStructure, }); }); From 0e3e8f039f5311e96cec1319f0153cb02f51a661 Mon Sep 17 00:00:00 2001 From: dependentmadani Date: Sun, 8 Sep 2024 17:40:45 +0100 Subject: [PATCH 08/31] update(tests): add different tests for the note parent structure --- src/presentation/http/router/note.test.ts | 127 ++++++++++++++++++++++ 1 file changed, 127 insertions(+) diff --git a/src/presentation/http/router/note.test.ts b/src/presentation/http/router/note.test.ts index 2187fb53..480302a1 100644 --- a/src/presentation/http/router/note.test.ts +++ b/src/presentation/http/router/note.test.ts @@ -518,6 +518,133 @@ describe('Note API', () => { expect(response?.json().message).toStrictEqual(expectedMessage); }); + + test('Returns one object of note parent structure by note public id with 200 status', async () => { + /** Create test user */ + const user = await global.db.insertUser(); + + /** Create test note - a parent note */ + const parentNote = await global.db.insertNote({ + creatorId: user.id, + }); + + /** Create test note - a child note */ + const childNote = await global.db.insertNote({ + creatorId: user.id, + }); + + /** Create test note settings */ + await global.db.insertNoteSetting({ + noteId: childNote.id, + isPublic: true, + }); + + /** Create test note relation */ + await global.db.insertNoteRelation({ + parentId: parentNote.id, + noteId: childNote.id, + }); + + const response = await global.api?.fakeRequest({ + method: 'GET', + url: `/note/${childNote.publicId}`, + }); + + expect(response?.statusCode).toBe(200); + + expect(response?.json()).toMatchObject({ + parentStructure: [ + { + noteId: parentNote.publicId, + content: parentNote.content, + }, + ], + }); + }); + + test('Returns multiple objects of note parent structure by note public id with 200 status', async () => { + /** Create test user */ + const user = await global.db.insertUser(); + + /** Create test note - a parent note */ + const parentNote = await global.db.insertNote({ + creatorId: user.id, + }); + + /** Create test note - a child note */ + const childNote = await global.db.insertNote({ + creatorId: user.id, + }); + + /** Create test note - a grandchild note */ + const grandchildNote = await global.db.insertNote({ + creatorId: user.id, + }); + + /** Create test note settings */ + await global.db.insertNoteSetting({ + noteId: grandchildNote.id, + isPublic: true, + }); + + /** Create test note relation */ + await global.db.insertNoteRelation({ + parentId: parentNote.id, + noteId: childNote.id, + }); + + await global.db.insertNoteRelation({ + parentId: childNote.id, + noteId: grandchildNote.id, + }); + + const response = await global.api?.fakeRequest({ + method: 'GET', + url: `/note/${grandchildNote.publicId}`, + }); + + expect(response?.statusCode).toBe(200); + + expect(response?.json()).toMatchObject({ + parentStructure: [ + { + noteId: childNote.publicId, + content: childNote.content, + }, + { + noteId: parentNote.publicId, + content: parentNote.content, + }, + ], + }); + }); + + test('Returns empty array of note parent structure by note public id with 200 status', async () => { + /** Create test user */ + const user = await global.db.insertUser(); + + /** Create test note */ + const note = await global.db.insertNote({ + creatorId: user.id, + }); + + /** Create test note settings */ + await global.db.insertNoteSetting({ + noteId: note.id, + isPublic: true, + }); + + const response = await global.api?.fakeRequest({ + method: 'GET', + url: `/note/${note.publicId}`, + }); + + expect(response?.statusCode).toBe(200); + + expect(response?.json()).toMatchObject({ + parentStructure: [], + }); + }); }); describe('PATCH note/:notePublicId ', () => { From 3adff283fbed66d021a8d19b506a82e69bc58e74 Mon Sep 17 00:00:00 2001 From: dependentmadani Date: Mon, 9 Sep 2024 18:26:12 +0100 Subject: [PATCH 09/31] update(tests): tests are now functional --- src/presentation/http/router/note.test.ts | 40 ++++++++++++++++++++--- src/presentation/http/router/note.ts | 14 ++++++++ 2 files changed, 49 insertions(+), 5 deletions(-) diff --git a/src/presentation/http/router/note.test.ts b/src/presentation/http/router/note.test.ts index 480302a1..c2cd9e90 100644 --- a/src/presentation/http/router/note.test.ts +++ b/src/presentation/http/router/note.test.ts @@ -519,10 +519,13 @@ describe('Note API', () => { expect(response?.json().message).toStrictEqual(expectedMessage); }); - test('Returns one object of note parent structure by note public id with 200 status', async () => { + test('Returns two objects of note parent structure by note public id with 200 status', async () => { /** Create test user */ const user = await global.db.insertUser(); + /** Create acces token for the user */ + const accessToken = global.auth(user.id); + /** Create test note - a parent note */ const parentNote = await global.db.insertNote({ creatorId: user.id, @@ -547,6 +550,9 @@ describe('Note API', () => { const response = await global.api?.fakeRequest({ method: 'GET', + headers: { + authorization: `Bearer ${accessToken}`, + }, url: `/note/${childNote.publicId}`, }); @@ -558,6 +564,10 @@ describe('Note API', () => { noteId: parentNote.publicId, content: parentNote.content, }, + { + noteId: childNote.publicId, + content: childNote.content, + }, ], }); }); @@ -566,6 +576,9 @@ describe('Note API', () => { /** Create test user */ const user = await global.db.insertUser(); + /** Create acces token for the user */ + const accessToken = global.auth(user.id); + /** Create test note - a parent note */ const parentNote = await global.db.insertNote({ creatorId: user.id, @@ -600,6 +613,9 @@ describe('Note API', () => { const response = await global.api?.fakeRequest({ method: 'GET', + headers: { + authorization: `Bearer ${accessToken}`, + }, url: `/note/${grandchildNote.publicId}`, }); @@ -607,22 +623,28 @@ describe('Note API', () => { expect(response?.json()).toMatchObject({ parentStructure: [ + { + noteId: parentNote.publicId, + content: parentNote.content, + }, { noteId: childNote.publicId, content: childNote.content, }, { - noteId: parentNote.publicId, - content: parentNote.content, + noteId: grandchildNote.publicId, + content: grandchildNote.content, }, ], }); }); - test('Returns empty array of note parent structure by note public id with 200 status', async () => { + test('Returns one object in array of note parent structure by note public id with 200 status', async () => { /** Create test user */ const user = await global.db.insertUser(); + /** Create acces token for the user */ + const accessToken = global.auth(user.id); /** Create test note */ const note = await global.db.insertNote({ creatorId: user.id, @@ -636,13 +658,21 @@ describe('Note API', () => { const response = await global.api?.fakeRequest({ method: 'GET', + headers: { + authorization: `Bearer ${accessToken}`, + }, url: `/note/${note.publicId}`, }); expect(response?.statusCode).toBe(200); expect(response?.json()).toMatchObject({ - parentStructure: [], + parentStructure: [ + { + noteId: note.publicId, + content: note.content, + }, + ], }); }); }); diff --git a/src/presentation/http/router/note.ts b/src/presentation/http/router/note.ts index 31ca6824..5207344e 100644 --- a/src/presentation/http/router/note.ts +++ b/src/presentation/http/router/note.ts @@ -124,6 +124,20 @@ const NoteRouter: FastifyPluginCallback = (fastify, opts, don $ref: 'EditorToolSchema', }, }, + parentStructure: { + type: 'array', + items: { + type: 'object', + properties: { + noteId: { + $ref: 'NoteSchema#/properties/id', + }, + content: { + $ref: 'NoteSchema#/properties/content', + }, + }, + }, + }, }, }, }, From d6c9ded047ef9e4c6cac0d92c0adf388a9bdc499 Mon Sep 17 00:00:00 2001 From: dependentmadani Date: Sun, 15 Sep 2024 00:17:52 +0100 Subject: [PATCH 10/31] update (parents note): modification based on the reviews, still tests to be fixed --- docker-compose.yml | 1 - src/domain/entities/note.ts | 22 ----- src/domain/service/note.ts | 6 +- src/presentation/http/router/note.test.ts | 72 ++++++++------- src/presentation/http/router/note.ts | 24 +++-- src/repository/index.ts | 3 +- src/repository/note.repository.ts | 11 +++ .../storage/postgres/orm/sequelize/note.ts | 88 +++++++++++++++++++ .../storage/postgres/orm/sequelize/teams.ts | 86 +----------------- src/repository/team.repository.ts | 12 +-- 10 files changed, 156 insertions(+), 169 deletions(-) diff --git a/docker-compose.yml b/docker-compose.yml index 6c9aacf3..d4ae844d 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -7,7 +7,6 @@ services: - 127.0.0.1:1337:1337 volumes: - ./app-config.yaml:/usr/app/app-config.yaml - - ./app-config.local.yaml:/usr/app/app-config.local.yaml restart: unless-stopped postgres: diff --git a/src/domain/entities/note.ts b/src/domain/entities/note.ts index 0537c851..b393a680 100644 --- a/src/domain/entities/note.ts +++ b/src/domain/entities/note.ts @@ -82,25 +82,3 @@ export interface Note { * Part of note entity used to create new note */ export type NoteCreationAttributes = Pick; - -/** - * This type represents only note's content - * Used for work with just note's content in web part - * This type would be used as a part of the structure, that represents all parent notes of the current note - */ -export type NoteIdAndContent = { - /** - * Note public id - */ - noteId: Note['publicId']; - - /** - * Note content - */ - content: Note['content']; -}; - -/** - * This type represents the structure, that represents all parent notes of the current note - */ -export type NoteParentsStructure = NoteIdAndContent[]; diff --git a/src/domain/service/note.ts b/src/domain/service/note.ts index fd073182..6ce2162f 100644 --- a/src/domain/service/note.ts +++ b/src/domain/service/note.ts @@ -1,4 +1,4 @@ -import type { Note, NoteInternalId, NoteParentsStructure, NotePublicId } from '@domain/entities/note.js'; +import type { Note, NoteInternalId, NotePublicId } from '@domain/entities/note.js'; import type NoteRepository from '@repository/note.repository.js'; import type NoteVisitsRepository from '@repository/noteVisits.repository.js'; import { createPublicId } from '@infrastructure/utils/id.js'; @@ -457,7 +457,7 @@ export default class NoteService { * @param userId - id of the user that is requesting the parent structure * @returns - array of notes that are parent structure of the note */ - public async getNoteParentStructure(noteId: NoteInternalId, userId: number): Promise { - return await this.teamRepository.getAllNotesParents(noteId, userId); + public async getNoteParentStructure(noteId: NoteInternalId, userId: number): Promise { + return await this.noteRepository.getAllNotesParents(noteId, userId); } } diff --git a/src/presentation/http/router/note.test.ts b/src/presentation/http/router/note.test.ts index c2cd9e90..de1f4563 100644 --- a/src/presentation/http/router/note.test.ts +++ b/src/presentation/http/router/note.test.ts @@ -519,7 +519,7 @@ describe('Note API', () => { expect(response?.json().message).toStrictEqual(expectedMessage); }); - test('Returns two objects of note parent structure by note public id with 200 status', async () => { + test('Returns two parents in case of relation between child and parent notes with 200 status', async () => { /** Create test user */ const user = await global.db.insertUser(); @@ -559,20 +559,22 @@ describe('Note API', () => { expect(response?.statusCode).toBe(200); expect(response?.json()).toMatchObject({ - parentStructure: [ - { - noteId: parentNote.publicId, - content: parentNote.content, - }, - { - noteId: childNote.publicId, - content: childNote.content, - }, - ], + parents: { + items: [ + { + id: parentNote.publicId, + content: parentNote.content, + }, + { + id: childNote.publicId, + content: childNote.content, + }, + ], + }, }); }); - test('Returns multiple objects of note parent structure by note public id with 200 status', async () => { + test('Returns multiple parents in case of multiple notes relations with user presence in team in each note with 200 status', async () => { /** Create test user */ const user = await global.db.insertUser(); @@ -622,24 +624,26 @@ describe('Note API', () => { expect(response?.statusCode).toBe(200); expect(response?.json()).toMatchObject({ - parentStructure: [ - { - noteId: parentNote.publicId, - content: parentNote.content, - }, - { - noteId: childNote.publicId, - content: childNote.content, - }, - { - noteId: grandchildNote.publicId, - content: grandchildNote.content, - }, - ], + parents: { + items: [ + { + id: parentNote.publicId, + content: parentNote.content + }, + { + id: childNote.publicId, + content: childNote.content + }, + { + id: grandchildNote.publicId, + content: grandchildNote.content + }, + ], + } }); }); - test('Returns one object in array of note parent structure by note public id with 200 status', async () => { + test('Returns one parent in case where there is no note relation with 200 status', async () => { /** Create test user */ const user = await global.db.insertUser(); @@ -667,12 +671,14 @@ describe('Note API', () => { expect(response?.statusCode).toBe(200); expect(response?.json()).toMatchObject({ - parentStructure: [ - { - noteId: note.publicId, - content: note.content, - }, - ], + parents: { + items: [ + { + id: note.publicId, + content: note.content, + }, + ], + } }); }); }); diff --git a/src/presentation/http/router/note.ts b/src/presentation/http/router/note.ts index 5207344e..5c9a307f 100644 --- a/src/presentation/http/router/note.ts +++ b/src/presentation/http/router/note.ts @@ -2,7 +2,7 @@ import type { FastifyPluginCallback } from 'fastify'; import type NoteService from '@domain/service/note.js'; import type NoteSettingsService from '@domain/service/noteSettings.js'; import type { ErrorResponse } from '@presentation/http/types/HttpResponse.js'; -import type { Note, NoteParentsStructure, NotePublicId } from '@domain/entities/note.js'; +import type { Note, NotePublicId } from '@domain/entities/note.js'; import useNoteResolver from '../middlewares/note/useNoteResolver.js'; import useNoteSettingsResolver from '../middlewares/noteSettings/useNoteSettingsResolver.js'; import useMemberRoleResolver from '../middlewares/noteSettings/useMemberRoleResolver.js'; @@ -12,6 +12,7 @@ import type NoteVisitsService from '@domain/service/noteVisits.js'; import type EditorToolsService from '@domain/service/editorTools.js'; import type EditorTool from '@domain/entities/editorTools.js'; import type { NoteHistoryMeta, NoteHistoryPublic, NoteHistoryRecord } from '@domain/entities/noteHistory.js'; +import { NoteList } from '@domain/entities/noteList.js'; /** * Interface for the note router. @@ -85,7 +86,7 @@ const NoteRouter: FastifyPluginCallback = (fastify, opts, don canEdit: boolean; }; tools: EditorTool[]; - parentStructure: NoteParentsStructure; + parents: NoteList; } | ErrorResponse; }>('/:notePublicId', { config: { @@ -124,16 +125,13 @@ const NoteRouter: FastifyPluginCallback = (fastify, opts, don $ref: 'EditorToolSchema', }, }, - parentStructure: { - type: 'array', - items: { - type: 'object', - properties: { - noteId: { - $ref: 'NoteSchema#/properties/id', - }, - content: { - $ref: 'NoteSchema#/properties/content', + parents: { + type: 'object', + properties: { + items: { + type: 'array', + items: { + $ref: 'NoteSchema', }, }, }, @@ -194,7 +192,7 @@ const NoteRouter: FastifyPluginCallback = (fastify, opts, don parentNote: parentNote, accessRights: { canEdit: canEdit }, tools: noteTools, - parentStructure: noteParentStructure, + parents: noteParentStructure, }); }); diff --git a/src/repository/index.ts b/src/repository/index.ts index d07f5aa1..d21da8ec 100644 --- a/src/repository/index.ts +++ b/src/repository/index.ts @@ -133,13 +133,14 @@ export async function init(orm: Orm, s3Config: S3StorageConfig): Promise { return await this.storage.getNoteListByUserId(id, offset, limit); } + + /** + * Get all notes parents based on note id and user id, by checking team access + * @param noteId : note id to get all its parents + * @param userId : user id to check access + * @returns an array of note parents objects containing public id and content + */ + public async getAllNotesParents(noteId: NoteInternalId, userId: number): Promise { + return await this.storage.getAllNoteParents(noteId, userId); + } } diff --git a/src/repository/storage/postgres/orm/sequelize/note.ts b/src/repository/storage/postgres/orm/sequelize/note.ts index d6a4d1ac..1a8baa10 100644 --- a/src/repository/storage/postgres/orm/sequelize/note.ts +++ b/src/repository/storage/postgres/orm/sequelize/note.ts @@ -6,6 +6,10 @@ import { UserModel } from '@repository/storage/postgres/orm/sequelize/user.js'; import type { NoteSettingsModel } from './noteSettings.js'; import type { NoteVisitsModel } from './noteVisits.js'; import type { NoteHistoryModel } from './noteHistory.js'; +import { NoteRelationsModel } from './noteRelations.js'; +import { notEmpty } from '@infrastructure/utils/empty.js'; +import { NoteList } from '@domain/entities/noteList.js'; +import { TeamsModel } from './teams.js'; /* eslint-disable @typescript-eslint/naming-convention */ @@ -75,6 +79,10 @@ export default class NoteSequelizeStorage { public historyModel: typeof NoteHistoryModel | null = null; + public noteRelationModel: typeof NoteRelationsModel | null = null; + + public teamModel: typeof TeamsModel | null = null; + /** * Database instance */ @@ -155,6 +163,22 @@ export default class NoteSequelizeStorage { }); }; + public createAssociationWithNoteRelationModel(model: ModelStatic): void { + + /** + * Create association with note relations + */ + this.noteRelationModel = model; + } + + public createAssociationWithTeamsModel(model: ModelStatic): void { + + /** + * Create association with teams + */ + this.teamModel = model; + } + /** * Insert note to database * @param options - note creation options @@ -323,4 +347,68 @@ export default class NoteSequelizeStorage { }, }); }; + + + /** + * Get all parent notes of a note that a user has access to, + * by checking the team access. + * @param noteId - the ID of the note. + * @param userId - the ID of the user. + */ + public async getAllNoteParents(noteId: NoteInternalId, userId: number): Promise { + if (!this.teamModel || !this.noteRelationModel) { + throw new Error(`${this.model !== null ? 'TeamStorage: Note relation model is not defined' : 'TeamStorage: Note model is not defined'}`); + } + + const parentNotes: NoteList = { items: [] }; + let currentNoteId: NoteInternalId | null = noteId; + /** + * Store notes that user can not access, to check the inherited team if has access + */ + let storeUnaccessibleNote: Note[] = []; + + while (currentNoteId != null) { + const teamMember = await this.teamModel.findOne({ + where: { + noteId: currentNoteId, + userId, + }, + include: { + model: this.model, + as: this.model.tableName, + required: true, + }, + }); + + if (teamMember && notEmpty(teamMember.notes)) { + if (storeUnaccessibleNote.length > 0) { + parentNotes.items.push(...storeUnaccessibleNote); + storeUnaccessibleNote = []; + } + parentNotes.items.push(teamMember.notes); + } else if (teamMember === null) { + const note = await this.model.findOne({ + where: { id: currentNoteId }, + }); + + if (note !== null) { + storeUnaccessibleNote.push(note); + } + } + + // Retrieve the parent note + const noteRelation: NoteRelationsModel | null = await this.noteRelationModel.findOne({ + where: { noteId: currentNoteId }, + }); + + if (noteRelation != null) { + currentNoteId = noteRelation.parentId; + } else { + currentNoteId = null; + } + } + + parentNotes.items.reverse(); + return parentNotes; + } } diff --git a/src/repository/storage/postgres/orm/sequelize/teams.ts b/src/repository/storage/postgres/orm/sequelize/teams.ts index bce9855f..824280f0 100644 --- a/src/repository/storage/postgres/orm/sequelize/teams.ts +++ b/src/repository/storage/postgres/orm/sequelize/teams.ts @@ -6,9 +6,7 @@ import type { Team, TeamMemberCreationAttributes, TeamMember } from '@domain/ent import { UserModel } from './user.js'; import { MemberRole } from '@domain/entities/team.js'; import type User from '@domain/entities/user.js'; -import type { NoteInternalId, NoteParentsStructure } from '@domain/entities/note.js'; -import type { NoteRelationsModel } from './noteRelations.js'; -import { isEmpty } from '@infrastructure/utils/empty.js'; +import type { NoteInternalId } from '@domain/entities/note.js'; /** * Class representing a teams model in database @@ -64,11 +62,6 @@ export default class TeamsSequelizeStorage { */ private userModel: typeof UserModel | null = null; - /** - * Note relation model instance - */ - private noteRelationModel: typeof NoteRelationsModel | null = null; - /** * Teams table name */ @@ -152,14 +145,6 @@ export default class TeamsSequelizeStorage { }); } - /** - * create association with note relations model - * @param model - initialized note relations model - */ - public createAssociationWithNoteRelationsModel(model: ModelStatic): void { - this.noteRelationModel = model; - } - /** * Create new team member membership * @param data - team membership data @@ -222,75 +207,6 @@ export default class TeamsSequelizeStorage { }); } - /** - * Get note parent structure - * @param noteId - the ID of the note. - * @param userId - the ID of the user. - */ - public async getAllNoteParents(noteId: NoteInternalId, userId: number): Promise { - if (!this.noteModel || !this.noteRelationModel) { - throw new Error(`${this.noteModel !== null ? 'TeamStorage: Note relation model is not defined' : 'TeamStorage: Note model is not defined'}`); - } - - const parentNotes: NoteParentsStructure = []; - let currentNoteId: NoteInternalId | null = noteId; - /** - * Store notes that user can not access, to check the inherited team if has access - */ - let storeUnaccessibleNote: NoteParentsStructure = []; - - while (currentNoteId != null) { - const teamMember = await this.model.findOne({ - where: { - noteId: currentNoteId, - userId, - }, - include: { - model: this.noteModel, - as: this.noteModel.tableName, - required: true, - attributes: ['publicId', 'content'], - }, - }); - - if (teamMember && !isEmpty(teamMember.notes)) { - if (storeUnaccessibleNote.length > 0) { - parentNotes.push(...storeUnaccessibleNote); - storeUnaccessibleNote = []; - } - parentNotes.push({ - noteId: teamMember.notes.publicId, - content: teamMember.notes.content, - }); - } else if (teamMember === null) { - const note = await this.noteModel.findOne({ - where: { id: currentNoteId }, - attributes: ['publicId', 'content'], - }); - - if (note !== null) { - storeUnaccessibleNote.push({ - noteId: note.publicId, - content: note.content, - }); - } - } - - // Retrieve the parent note - const noteRelation: NoteRelationsModel | null = await this.noteRelationModel.findOne({ - where: { noteId: currentNoteId }, - }); - - if (noteRelation != null) { - currentNoteId = noteRelation.parentId; - } else { - currentNoteId = null; - } - } - - return parentNotes.reverse(); - } - /** * Remove team member by id * @param userId - id of team member diff --git a/src/repository/team.repository.ts b/src/repository/team.repository.ts index c426c35f..fb01f0d3 100644 --- a/src/repository/team.repository.ts +++ b/src/repository/team.repository.ts @@ -1,6 +1,6 @@ import type TeamStorage from '@repository/storage/team.storage.js'; import type { Team, TeamMember, TeamMemberCreationAttributes, MemberRole } from '@domain/entities/team.js'; -import type { NoteInternalId, NoteParentsStructure } from '@domain/entities/note.js'; +import type { NoteInternalId } from '@domain/entities/note.js'; import type User from '@domain/entities/user.js'; /** @@ -57,16 +57,6 @@ export default class TeamRepository { return await this.storage.getTeamMembersWithUserInfoByNoteId(noteId); }; - /** - * Get all notes parents based on note id and user id, by checking team access - * @param noteId : note id to get all its parents - * @param userId : user id to check access - * @returns an array of note parents objects containing public id and content - */ - public async getAllNotesParents(noteId: NoteInternalId, userId: number): Promise { - return await this.storage.getAllNoteParents(noteId, userId); - } - /** * Remove team member by id * @param userId - id of the team member From 6bdc56a49ecc5ac498a66de89851fb0ae5a81480 Mon Sep 17 00:00:00 2001 From: dependentmadani Date: Sun, 15 Sep 2024 00:21:18 +0100 Subject: [PATCH 11/31] update: fix lint --- src/presentation/http/router/note.test.ts | 10 +++++----- src/presentation/http/router/note.ts | 2 +- src/repository/note.repository.ts | 2 +- .../storage/postgres/orm/sequelize/note.ts | 12 +++++------- 4 files changed, 12 insertions(+), 14 deletions(-) diff --git a/src/presentation/http/router/note.test.ts b/src/presentation/http/router/note.test.ts index de1f4563..cee8788a 100644 --- a/src/presentation/http/router/note.test.ts +++ b/src/presentation/http/router/note.test.ts @@ -628,18 +628,18 @@ describe('Note API', () => { items: [ { id: parentNote.publicId, - content: parentNote.content + content: parentNote.content, }, { id: childNote.publicId, - content: childNote.content + content: childNote.content, }, { id: grandchildNote.publicId, - content: grandchildNote.content + content: grandchildNote.content, }, ], - } + }, }); }); @@ -678,7 +678,7 @@ describe('Note API', () => { content: note.content, }, ], - } + }, }); }); }); diff --git a/src/presentation/http/router/note.ts b/src/presentation/http/router/note.ts index 5c9a307f..f9f76be4 100644 --- a/src/presentation/http/router/note.ts +++ b/src/presentation/http/router/note.ts @@ -12,7 +12,7 @@ import type NoteVisitsService from '@domain/service/noteVisits.js'; import type EditorToolsService from '@domain/service/editorTools.js'; import type EditorTool from '@domain/entities/editorTools.js'; import type { NoteHistoryMeta, NoteHistoryPublic, NoteHistoryRecord } from '@domain/entities/noteHistory.js'; -import { NoteList } from '@domain/entities/noteList.js'; +import type { NoteList } from '@domain/entities/noteList.js'; /** * Interface for the note router. diff --git a/src/repository/note.repository.ts b/src/repository/note.repository.ts index b0f0001f..cb4769c7 100644 --- a/src/repository/note.repository.ts +++ b/src/repository/note.repository.ts @@ -1,5 +1,5 @@ import type { Note, NoteCreationAttributes, NoteInternalId, NotePublicId } from '@domain/entities/note.js'; -import { NoteList } from '@domain/entities/noteList.js'; +import type { NoteList } from '@domain/entities/noteList.js'; import type NoteStorage from '@repository/storage/note.storage.js'; /** diff --git a/src/repository/storage/postgres/orm/sequelize/note.ts b/src/repository/storage/postgres/orm/sequelize/note.ts index 1a8baa10..d78c9884 100644 --- a/src/repository/storage/postgres/orm/sequelize/note.ts +++ b/src/repository/storage/postgres/orm/sequelize/note.ts @@ -6,10 +6,10 @@ import { UserModel } from '@repository/storage/postgres/orm/sequelize/user.js'; import type { NoteSettingsModel } from './noteSettings.js'; import type { NoteVisitsModel } from './noteVisits.js'; import type { NoteHistoryModel } from './noteHistory.js'; -import { NoteRelationsModel } from './noteRelations.js'; +import type { NoteRelationsModel } from './noteRelations.js'; import { notEmpty } from '@infrastructure/utils/empty.js'; -import { NoteList } from '@domain/entities/noteList.js'; -import { TeamsModel } from './teams.js'; +import type { NoteList } from '@domain/entities/noteList.js'; +import type { TeamsModel } from './teams.js'; /* eslint-disable @typescript-eslint/naming-convention */ @@ -164,7 +164,6 @@ export default class NoteSequelizeStorage { }; public createAssociationWithNoteRelationModel(model: ModelStatic): void { - /** * Create association with note relations */ @@ -172,10 +171,9 @@ export default class NoteSequelizeStorage { } public createAssociationWithTeamsModel(model: ModelStatic): void { - /** * Create association with teams - */ + */ this.teamModel = model; } @@ -348,7 +346,6 @@ export default class NoteSequelizeStorage { }); }; - /** * Get all parent notes of a note that a user has access to, * by checking the team access. @@ -409,6 +406,7 @@ export default class NoteSequelizeStorage { } parentNotes.items.reverse(); + return parentNotes; } } From 6cf3d4cbb5cf62486dbedae9dfc42480c10e1cfa Mon Sep 17 00:00:00 2001 From: dependentmadani Date: Sun, 15 Sep 2024 16:43:05 +0100 Subject: [PATCH 12/31] update (note parents): few modification in the return value type, add a test, tests work in progress --- src/domain/service/note.ts | 3 +- src/presentation/http/router/note.test.ts | 127 +++++++++++++----- src/presentation/http/router/note.ts | 14 +- src/repository/note.repository.ts | 4 +- .../storage/postgres/orm/sequelize/note.ts | 24 +++- 5 files changed, 118 insertions(+), 54 deletions(-) diff --git a/src/domain/service/note.ts b/src/domain/service/note.ts index 6ce2162f..688b95a2 100644 --- a/src/domain/service/note.ts +++ b/src/domain/service/note.ts @@ -10,6 +10,7 @@ import type { NoteList } from '@domain/entities/noteList.js'; import type NoteHistoryRepository from '@repository/noteHistory.repository.js'; import type { NoteHistoryMeta, NoteHistoryRecord, NoteHistoryPublic } from '@domain/entities/noteHistory.js'; import type TeamRepository from '@repository/team.repository.js'; +import type { NotePublic } from '@domain/entities/notePublic.js'; /** * Note service @@ -457,7 +458,7 @@ export default class NoteService { * @param userId - id of the user that is requesting the parent structure * @returns - array of notes that are parent structure of the note */ - public async getNoteParentStructure(noteId: NoteInternalId, userId: number): Promise { + public async getNoteParentStructure(noteId: NoteInternalId, userId: number): Promise { return await this.noteRepository.getAllNotesParents(noteId, userId); } } diff --git a/src/presentation/http/router/note.test.ts b/src/presentation/http/router/note.test.ts index cee8788a..2bc1d26a 100644 --- a/src/presentation/http/router/note.test.ts +++ b/src/presentation/http/router/note.test.ts @@ -559,18 +559,16 @@ describe('Note API', () => { expect(response?.statusCode).toBe(200); expect(response?.json()).toMatchObject({ - parents: { - items: [ - { - id: parentNote.publicId, - content: parentNote.content, - }, - { - id: childNote.publicId, - content: childNote.content, - }, - ], - }, + parents: [ + { + id: parentNote.publicId, + content: parentNote.content, + }, + { + id: childNote.publicId, + content: childNote.content, + }, + ], }); }); @@ -624,22 +622,20 @@ describe('Note API', () => { expect(response?.statusCode).toBe(200); expect(response?.json()).toMatchObject({ - parents: { - items: [ - { - id: parentNote.publicId, - content: parentNote.content, - }, - { - id: childNote.publicId, - content: childNote.content, - }, - { - id: grandchildNote.publicId, - content: grandchildNote.content, - }, - ], - }, + parents: [ + { + id: parentNote.publicId, + content: parentNote.content, + }, + { + id: childNote.publicId, + content: childNote.content, + }, + { + id: grandchildNote.publicId, + content: grandchildNote.content, + }, + ], }); }); @@ -671,14 +667,75 @@ describe('Note API', () => { expect(response?.statusCode).toBe(200); expect(response?.json()).toMatchObject({ - parents: { - items: [ - { - id: note.publicId, - content: note.content, - }, - ], + parents: [ + { + id: note.publicId, + content: note.content, + }, + ], + }); + }); + + test('Returns mutiple parents in case where user is not in the team of a note with 200 status', async () => { + /** Create test user */ + const user1 = await global.db.insertUser(); + const user2 = await global.db.insertUser(); + + /** Create acces token for the user */ + const accessToken = global.auth(user1.id); + /** Create test note */ + const grandChildNote = await global.db.insertNote({ + creatorId: user1.id, + }); + + const childNote = await global.db.insertNote({ + creatorId: user2.id, + }); + + const parentNote = await global.db.insertNote({ + creatorId: user1.id, + }); + + await global.db.insertNoteSetting({ + noteId: grandChildNote.id, + isPublic: true, + }); + + await global.db.insertNoteRelation({ + parentId: parentNote.id, + noteId: childNote.id, + }); + + await global.db.insertNoteRelation({ + parentId: childNote.id, + noteId: grandChildNote.id, + }); + + const response = await global.api?.fakeRequest({ + method: 'GET', + headers: { + authorization: `Bearer ${accessToken}`, }, + url: `/note/${grandChildNote.publicId}`, + }); + + expect(response?.statusCode).toBe(200); + + expect(response?.json()).toMatchObject({ + parents: [ + { + id: parentNote.publicId, + content: parentNote.content, + }, + { + id: childNote.publicId, + content: childNote.content, + }, + { + id: grandChildNote.publicId, + content: grandChildNote.content, + }, + ], }); }); }); diff --git a/src/presentation/http/router/note.ts b/src/presentation/http/router/note.ts index f9f76be4..fa00fe65 100644 --- a/src/presentation/http/router/note.ts +++ b/src/presentation/http/router/note.ts @@ -12,7 +12,6 @@ import type NoteVisitsService from '@domain/service/noteVisits.js'; import type EditorToolsService from '@domain/service/editorTools.js'; import type EditorTool from '@domain/entities/editorTools.js'; import type { NoteHistoryMeta, NoteHistoryPublic, NoteHistoryRecord } from '@domain/entities/noteHistory.js'; -import type { NoteList } from '@domain/entities/noteList.js'; /** * Interface for the note router. @@ -86,7 +85,7 @@ const NoteRouter: FastifyPluginCallback = (fastify, opts, don canEdit: boolean; }; tools: EditorTool[]; - parents: NoteList; + parents: NotePublic[]; } | ErrorResponse; }>('/:notePublicId', { config: { @@ -126,14 +125,9 @@ const NoteRouter: FastifyPluginCallback = (fastify, opts, don }, }, parents: { - type: 'object', - properties: { - items: { - type: 'array', - items: { - $ref: 'NoteSchema', - }, - }, + type: 'array', + items: { + $ref: 'NoteSchema', }, }, }, diff --git a/src/repository/note.repository.ts b/src/repository/note.repository.ts index cb4769c7..25440258 100644 --- a/src/repository/note.repository.ts +++ b/src/repository/note.repository.ts @@ -1,5 +1,5 @@ import type { Note, NoteCreationAttributes, NoteInternalId, NotePublicId } from '@domain/entities/note.js'; -import type { NoteList } from '@domain/entities/noteList.js'; +import type { NotePublic } from '@domain/entities/notePublic.js'; import type NoteStorage from '@repository/storage/note.storage.js'; /** @@ -89,7 +89,7 @@ export default class NoteRepository { * @param userId : user id to check access * @returns an array of note parents objects containing public id and content */ - public async getAllNotesParents(noteId: NoteInternalId, userId: number): Promise { + public async getAllNotesParents(noteId: NoteInternalId, userId: number): Promise { return await this.storage.getAllNoteParents(noteId, userId); } } diff --git a/src/repository/storage/postgres/orm/sequelize/note.ts b/src/repository/storage/postgres/orm/sequelize/note.ts index d78c9884..200b6b70 100644 --- a/src/repository/storage/postgres/orm/sequelize/note.ts +++ b/src/repository/storage/postgres/orm/sequelize/note.ts @@ -8,8 +8,8 @@ import type { NoteVisitsModel } from './noteVisits.js'; import type { NoteHistoryModel } from './noteHistory.js'; import type { NoteRelationsModel } from './noteRelations.js'; import { notEmpty } from '@infrastructure/utils/empty.js'; -import type { NoteList } from '@domain/entities/noteList.js'; import type { TeamsModel } from './teams.js'; +import type { NotePublic } from '@domain/entities/notePublic.js'; /* eslint-disable @typescript-eslint/naming-convention */ @@ -352,12 +352,12 @@ export default class NoteSequelizeStorage { * @param noteId - the ID of the note. * @param userId - the ID of the user. */ - public async getAllNoteParents(noteId: NoteInternalId, userId: number): Promise { + public async getAllNoteParents(noteId: NoteInternalId, userId: number): Promise { if (!this.teamModel || !this.noteRelationModel) { throw new Error(`${this.model !== null ? 'TeamStorage: Note relation model is not defined' : 'TeamStorage: Note model is not defined'}`); } - const parentNotes: NoteList = { items: [] }; + const parentNotes: NotePublic[] = []; let currentNoteId: NoteInternalId | null = noteId; /** * Store notes that user can not access, to check the inherited team if has access @@ -379,10 +379,22 @@ export default class NoteSequelizeStorage { if (teamMember && notEmpty(teamMember.notes)) { if (storeUnaccessibleNote.length > 0) { - parentNotes.items.push(...storeUnaccessibleNote); + parentNotes.push(...storeUnaccessibleNote.map(note => ({ + id: note.publicId, + content: note.content, + updatedAt: note.updatedAt, + createdAt: note.createdAt, + creatorId: note.creatorId, + }))); storeUnaccessibleNote = []; } - parentNotes.items.push(teamMember.notes); + parentNotes.push({ + id: teamMember.notes.publicId, + content: teamMember.notes.content, + updatedAt: teamMember.notes.updatedAt, + createdAt: teamMember.notes.createdAt, + creatorId: teamMember.notes.creatorId, + }); } else if (teamMember === null) { const note = await this.model.findOne({ where: { id: currentNoteId }, @@ -405,7 +417,7 @@ export default class NoteSequelizeStorage { } } - parentNotes.items.reverse(); + parentNotes.reverse(); return parentNotes; } From 1809ca187fcbf66320ac4d5ebc14593169c380ad Mon Sep 17 00:00:00 2001 From: dependentmadani Date: Sun, 15 Sep 2024 21:14:48 +0100 Subject: [PATCH 13/31] update: add a test --- src/presentation/http/router/note.test.ts | 75 ++++++++++++++++++++++- 1 file changed, 74 insertions(+), 1 deletion(-) diff --git a/src/presentation/http/router/note.test.ts b/src/presentation/http/router/note.test.ts index 2bc1d26a..e4104e5e 100644 --- a/src/presentation/http/router/note.test.ts +++ b/src/presentation/http/router/note.test.ts @@ -683,24 +683,97 @@ describe('Note API', () => { /** Create acces token for the user */ const accessToken = global.auth(user1.id); - /** Create test note */ + + /** Create test base note */ const grandChildNote = await global.db.insertNote({ creatorId: user1.id, }); + /** Create base note parent */ const childNote = await global.db.insertNote({ creatorId: user2.id, }); + /** Create base note grand parent */ const parentNote = await global.db.insertNote({ creatorId: user1.id, }); + /** Create base note settings */ await global.db.insertNoteSetting({ noteId: grandChildNote.id, isPublic: true, }); + /** Create note relations */ + await global.db.insertNoteRelation({ + parentId: parentNote.id, + noteId: childNote.id, + }); + + await global.db.insertNoteRelation({ + parentId: childNote.id, + noteId: grandChildNote.id, + }); + + const response = await global.api?.fakeRequest({ + method: 'GET', + headers: { + authorization: `Bearer ${accessToken}`, + }, + url: `/note/${grandChildNote.publicId}`, + }); + + expect(response?.statusCode).toBe(200); + + expect(response?.json()).toMatchObject({ + parents: [ + { + id: parentNote.publicId, + content: parentNote.content, + }, + { + id: childNote.publicId, + content: childNote.content, + }, + { + id: grandChildNote.publicId, + content: grandChildNote.content, + }, + ], + }); + }); + + test('Returns multiple parents in case when note is not public with 200 status', async () => { + /** Create test user */ + const user1 = await global.db.insertUser(); + const user2 = await global.db.insertUser(); + + /** Create acces token for the user */ + const accessToken = global.auth(user1.id); + + /** Create test base note */ + const grandChildNote = await global.db.insertNote({ + creatorId: user1.id, + }); + + /** Create base note parent */ + const childNote = await global.db.insertNote({ + creatorId: user2.id, + }); + + /** Create base note grand parent */ + const parentNote = await global.db.insertNote({ + creatorId: user1.id, + }); + + /** Create base note settings */ + await global.db.insertNoteSetting({ + noteId: grandChildNote.id, + isPublic: false, + }); + + /** Create note relations */ await global.db.insertNoteRelation({ parentId: parentNote.id, noteId: childNote.id, From ca7a9a2a03c68cf9f21d295cc1f8f91be075bed1 Mon Sep 17 00:00:00 2001 From: dependentmadani Date: Wed, 18 Sep 2024 22:59:07 +0100 Subject: [PATCH 14/31] update (note parents): modifications based on reviews, use test.each in tests instead of individual test cases --- docker-compose.yml | 2 + src/domain/index.ts | 2 +- src/domain/service/note.ts | 12 +- src/presentation/http/router/note.test.ts | 417 +++++++--------------- src/repository/note.repository.ts | 2 +- 5 files changed, 140 insertions(+), 295 deletions(-) diff --git a/docker-compose.yml b/docker-compose.yml index d4ae844d..c741bc60 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -1,3 +1,5 @@ +version: '3.2' + services: api: build: diff --git a/src/domain/index.ts b/src/domain/index.ts index 7a0abf0b..44ed849b 100644 --- a/src/domain/index.ts +++ b/src/domain/index.ts @@ -63,7 +63,7 @@ export function init(repositories: Repositories, appConfig: AppConfig): DomainSe /** * @todo use shared methods for uncoupling repositories unrelated to note service */ - const noteService = new NoteService(repositories.noteRepository, repositories.noteRelationsRepository, repositories.noteVisitsRepository, repositories.editorToolsRepository, repositories.noteHistoryRepository, repositories.teamRepository); + const noteService = new NoteService(repositories.noteRepository, repositories.noteRelationsRepository, repositories.noteVisitsRepository, repositories.editorToolsRepository, repositories.noteHistoryRepository); const noteVisitsService = new NoteVisitsService(repositories.noteVisitsRepository); const authService = new AuthService( appConfig.auth.accessSecret, diff --git a/src/domain/service/note.ts b/src/domain/service/note.ts index 688b95a2..7571169d 100644 --- a/src/domain/service/note.ts +++ b/src/domain/service/note.ts @@ -9,7 +9,6 @@ import type User from '@domain/entities/user.js'; import type { NoteList } from '@domain/entities/noteList.js'; import type NoteHistoryRepository from '@repository/noteHistory.repository.js'; import type { NoteHistoryMeta, NoteHistoryRecord, NoteHistoryPublic } from '@domain/entities/noteHistory.js'; -import type TeamRepository from '@repository/team.repository.js'; import type { NotePublic } from '@domain/entities/notePublic.js'; /** @@ -41,11 +40,6 @@ export default class NoteService { */ public noteHistoryRepository: NoteHistoryRepository; - /** - * Team repository - */ - public teamRepository: TeamRepository; - /** * Number of the notes to be displayed on one page * it is used to calculate offset and limit for getting notes that the user has recently opened @@ -64,15 +58,13 @@ export default class NoteService { * @param noteVisitsRepository - note visits repository * @param editorToolsRepository - editor tools repositoryn * @param noteHistoryRepository - note history repository - * @param teamRepository - team note repository */ - constructor(noteRepository: NoteRepository, noteRelationsRepository: NoteRelationsRepository, noteVisitsRepository: NoteVisitsRepository, editorToolsRepository: EditorToolsRepository, noteHistoryRepository: NoteHistoryRepository, teamRepository: TeamRepository) { + constructor(noteRepository: NoteRepository, noteRelationsRepository: NoteRelationsRepository, noteVisitsRepository: NoteVisitsRepository, editorToolsRepository: EditorToolsRepository, noteHistoryRepository: NoteHistoryRepository) { this.noteRepository = noteRepository; this.noteRelationsRepository = noteRelationsRepository; this.noteVisitsRepository = noteVisitsRepository; this.editorToolsRepository = editorToolsRepository; this.noteHistoryRepository = noteHistoryRepository; - this.teamRepository = teamRepository; } /** @@ -459,6 +451,6 @@ export default class NoteService { * @returns - array of notes that are parent structure of the note */ public async getNoteParentStructure(noteId: NoteInternalId, userId: number): Promise { - return await this.noteRepository.getAllNotesParents(noteId, userId); + return await this.noteRepository.getNoteParents(noteId, userId); } } diff --git a/src/presentation/http/router/note.test.ts b/src/presentation/http/router/note.test.ts index e4104e5e..6dd7ede6 100644 --- a/src/presentation/http/router/note.test.ts +++ b/src/presentation/http/router/note.test.ts @@ -1,6 +1,7 @@ import { MemberRole } from '@domain/entities/team.js'; import { describe, test, expect, beforeEach } from 'vitest'; import type User from '@domain/entities/user.js'; +import type { Note } from '@domain/entities/note.js'; describe('Note API', () => { /** @@ -180,6 +181,47 @@ describe('Note API', () => { }); describe('GET note/:notePublicId ', () => { + let context: { + user: User; + parentNote: Note; + childNote: Note; + differentChildNote: Note; + grandChildNote: Note; + } = { + user: {} as User, + parentNote: {} as Note, + childNote: {} as Note, + differentChildNote: {} as Note, + grandChildNote: {} as Note, + }; + + beforeEach(async () => { + /** Create test user */ + context.user = await global.db.insertUser(); + + const user2 = await global.db.insertUser(); + + /** Create test note - a parent note */ + context.parentNote = await global.db.insertNote({ + creatorId: context.user.id, + }); + + /** Create test note - a child note */ + context.childNote = await global.db.insertNote({ + creatorId: context.user.id, + }); + + /** Create test note - create note with different user */ + context.differentChildNote = await global.db.insertNote({ + creatorId: user2.id, + }); + + /** Create test note - a grandchild note */ + context.grandChildNote = await global.db.insertNote({ + creatorId: context.user.id, + }); + }); + test.each([ /** Returns 200 if user is team member with a Read role */ { @@ -519,297 +561,106 @@ describe('Note API', () => { expect(response?.json().message).toStrictEqual(expectedMessage); }); - test('Returns two parents in case of relation between child and parent notes with 200 status', async () => { - /** Create test user */ - const user = await global.db.insertUser(); - - /** Create acces token for the user */ - const accessToken = global.auth(user.id); - - /** Create test note - a parent note */ - const parentNote = await global.db.insertNote({ - creatorId: user.id, - }); - - /** Create test note - a child note */ - const childNote = await global.db.insertNote({ - creatorId: user.id, - }); - - /** Create test note settings */ - await global.db.insertNoteSetting({ - noteId: childNote.id, + test.each([ + /** Returns two parents in case of relation between child and parent notes with 200 status */ + { + numberOfNotes: 2, + userNoteCreationDifferent: false, isPublic: true, - }); - - /** Create test note relation */ - await global.db.insertNoteRelation({ - parentId: parentNote.id, - noteId: childNote.id, - }); - - const response = await global.api?.fakeRequest({ - method: 'GET', - headers: { - authorization: `Bearer ${accessToken}`, - }, - url: `/note/${childNote.publicId}`, - }); - - expect(response?.statusCode).toBe(200); - - expect(response?.json()).toMatchObject({ - parents: [ - { - id: parentNote.publicId, - content: parentNote.content, - }, - { - id: childNote.publicId, - content: childNote.content, - }, - ], - }); - }); - - test('Returns multiple parents in case of multiple notes relations with user presence in team in each note with 200 status', async () => { - /** Create test user */ - const user = await global.db.insertUser(); - - /** Create acces token for the user */ - const accessToken = global.auth(user.id); - - /** Create test note - a parent note */ - const parentNote = await global.db.insertNote({ - creatorId: user.id, - }); - - /** Create test note - a child note */ - const childNote = await global.db.insertNote({ - creatorId: user.id, - }); - - /** Create test note - a grandchild note */ - const grandchildNote = await global.db.insertNote({ - creatorId: user.id, - }); - - /** Create test note settings */ - await global.db.insertNoteSetting({ - noteId: grandchildNote.id, + }, + /** Returns multiple parents in case of multiple notes relations with user presence in team in each note with 200 status */ + { + numberOfNotes: 3, + userNoteCreationDifferent: false, isPublic: true, - }); - - /** Create test note relation */ - await global.db.insertNoteRelation({ - parentId: parentNote.id, - noteId: childNote.id, - }); - - await global.db.insertNoteRelation({ - parentId: childNote.id, - noteId: grandchildNote.id, - }); - - const response = await global.api?.fakeRequest({ - method: 'GET', - headers: { - authorization: `Bearer ${accessToken}`, - }, - url: `/note/${grandchildNote.publicId}`, - }); - - expect(response?.statusCode).toBe(200); - - expect(response?.json()).toMatchObject({ - parents: [ - { - id: parentNote.publicId, - content: parentNote.content, - }, - { - id: childNote.publicId, - content: childNote.content, - }, - { - id: grandchildNote.publicId, - content: grandchildNote.content, - }, - ], - }); - }); - - test('Returns one parent in case where there is no note relation with 200 status', async () => { - /** Create test user */ - const user = await global.db.insertUser(); - - /** Create acces token for the user */ - const accessToken = global.auth(user.id); - /** Create test note */ - const note = await global.db.insertNote({ - creatorId: user.id, - }); - - /** Create test note settings */ - await global.db.insertNoteSetting({ - noteId: note.id, + }, + /** Returns one parent in case where there is no note relation with 200 status */ + { + numberOfNotes: 1, + userNoteCreationDifferent: false, isPublic: true, - }); - - const response = await global.api?.fakeRequest({ - method: 'GET', - headers: { - authorization: `Bearer ${accessToken}`, - }, - url: `/note/${note.publicId}`, - }); - - expect(response?.statusCode).toBe(200); - - expect(response?.json()).toMatchObject({ - parents: [ - { - id: note.publicId, - content: note.content, - }, - ], - }); - }); - - test('Returns mutiple parents in case where user is not in the team of a note with 200 status', async () => { - /** Create test user */ - const user1 = await global.db.insertUser(); - const user2 = await global.db.insertUser(); - - /** Create acces token for the user */ - const accessToken = global.auth(user1.id); - - /** Create test base note */ - const grandChildNote = await global.db.insertNote({ - creatorId: user1.id, - }); - - /** Create base note parent */ - const childNote = await global.db.insertNote({ - creatorId: user2.id, - }); - - /** Create base note grand parent */ - const parentNote = await global.db.insertNote({ - creatorId: user1.id, - }); - - /** Create base note settings */ - await global.db.insertNoteSetting({ - noteId: grandChildNote.id, + }, + /** Returns mutiple parents in case where user is not in the team of a note with 200 status */ + { + numberOfNotes: 3, + userNoteCreationDifferent: true, isPublic: true, - }); - - /** Create note relations */ - await global.db.insertNoteRelation({ - parentId: parentNote.id, - noteId: childNote.id, - }); - - await global.db.insertNoteRelation({ - parentId: childNote.id, - noteId: grandChildNote.id, - }); - - const response = await global.api?.fakeRequest({ - method: 'GET', - headers: { - authorization: `Bearer ${accessToken}`, - }, - url: `/note/${grandChildNote.publicId}`, - }); - - expect(response?.statusCode).toBe(200); - - expect(response?.json()).toMatchObject({ - parents: [ - { - id: parentNote.publicId, - content: parentNote.content, - }, - { - id: childNote.publicId, - content: childNote.content, - }, - { - id: grandChildNote.publicId, - content: grandChildNote.content, - }, - ], - }); - }); - - test('Returns multiple parents in case when note is not public with 200 status', async () => { - /** Create test user */ - const user1 = await global.db.insertUser(); - const user2 = await global.db.insertUser(); - - /** Create acces token for the user */ - const accessToken = global.auth(user1.id); - - /** Create test base note */ - const grandChildNote = await global.db.insertNote({ - creatorId: user1.id, - }); - - /** Create base note parent */ - const childNote = await global.db.insertNote({ - creatorId: user2.id, - }); - - /** Create base note grand parent */ - const parentNote = await global.db.insertNote({ - creatorId: user1.id, - }); - - /** Create base note settings */ - await global.db.insertNoteSetting({ - noteId: grandChildNote.id, + }, + /** Returns multiple parents in case when note is not public with 200 status */ + { + numberOfNotes: 3, + userNoteCreationDifferent: true, isPublic: false, - }); - - /** Create note relations */ - await global.db.insertNoteRelation({ - parentId: parentNote.id, - noteId: childNote.id, - }); - - await global.db.insertNoteRelation({ - parentId: childNote.id, - noteId: grandChildNote.id, - }); + }, + ])('Get note parents in different scenarios', async ({ numberOfNotes, userNoteCreationDifferent, isPublic }) => { + /** The variable that stores the expected result */ + let expectedOutput: { id: string; content: object }[] = []; + + if (context !== undefined) { + /** Create acces token for the user */ + const accessToken = global.auth(context.user.id); + let noteId = context.parentNote.id; + let notePublicId = context.parentNote.publicId; + + if (numberOfNotes == 2 && !userNoteCreationDifferent) { + noteId = context.childNote.id; + notePublicId = context.childNote.publicId; + } else if (numberOfNotes == 2 && userNoteCreationDifferent) { + noteId = context.differentChildNote.id; + notePublicId = context.differentChildNote.publicId; + } else if (numberOfNotes == 3) { + noteId = context.grandChildNote.id; + notePublicId = context.grandChildNote.publicId; + } + + /** Create test note settings */ + await global.db.insertNoteSetting({ + noteId: noteId, + isPublic: isPublic, + }); - const response = await global.api?.fakeRequest({ - method: 'GET', - headers: { - authorization: `Bearer ${accessToken}`, - }, - url: `/note/${grandChildNote.publicId}`, - }); + for (let i = 0; i < numberOfNotes - 1; i++) { + /** Create test note relation */ + await global.db.insertNoteRelation({ + parentId: i == 0 ? context.parentNote.id : (userNoteCreationDifferent ? context.differentChildNote.id : context.childNote.id), + noteId: i == 0 ? (userNoteCreationDifferent ? context.differentChildNote.id : context.childNote.id) : context.grandChildNote.id, + }); + } + + /** The expected output that should be returned in the response */ + for (let i = 0; i < numberOfNotes; i++) { + if (i == 0) { + expectedOutput.push({ + id: context.parentNote.publicId, + content: context.parentNote.content, + }); + } else if (i == 1) { + expectedOutput.push({ + id: userNoteCreationDifferent ? context.differentChildNote.publicId : context.childNote.publicId, + content: userNoteCreationDifferent ? context.differentChildNote.content : context.childNote.content, + }); + } else { + expectedOutput.push({ + id: context.grandChildNote.publicId, + content: context.grandChildNote.content, + }); + } + } + + const response = await global.api?.fakeRequest({ + method: 'GET', + headers: { + authorization: `Bearer ${accessToken}`, + }, + url: `/note/${notePublicId}`, + }); - expect(response?.statusCode).toBe(200); + expect(response?.statusCode).toBe(200); - expect(response?.json()).toMatchObject({ - parents: [ - { - id: parentNote.publicId, - content: parentNote.content, - }, - { - id: childNote.publicId, - content: childNote.content, - }, - { - id: grandChildNote.publicId, - content: grandChildNote.content, - }, - ], - }); + expect(response?.json()).toMatchObject({ + parents: expectedOutput, + }); + } }); }); diff --git a/src/repository/note.repository.ts b/src/repository/note.repository.ts index 25440258..0fc7bcbc 100644 --- a/src/repository/note.repository.ts +++ b/src/repository/note.repository.ts @@ -89,7 +89,7 @@ export default class NoteRepository { * @param userId : user id to check access * @returns an array of note parents objects containing public id and content */ - public async getAllNotesParents(noteId: NoteInternalId, userId: number): Promise { + public async getNoteParents(noteId: NoteInternalId, userId: number): Promise { return await this.storage.getAllNoteParents(noteId, userId); } } From e14d9485201e414754ef9f46721deddd2e9ea6b0 Mon Sep 17 00:00:00 2001 From: dependentmadani Date: Fri, 20 Sep 2024 00:36:01 +0100 Subject: [PATCH 15/31] update (parent note test): modification based on previous reviews --- src/presentation/http/router/note.test.ts | 167 ++++++++++++++++------ 1 file changed, 126 insertions(+), 41 deletions(-) diff --git a/src/presentation/http/router/note.test.ts b/src/presentation/http/router/note.test.ts index 6dd7ede6..1394babb 100644 --- a/src/presentation/http/router/note.test.ts +++ b/src/presentation/http/router/note.test.ts @@ -183,12 +183,14 @@ describe('Note API', () => { describe('GET note/:notePublicId ', () => { let context: { user: User; + user2: User; parentNote: Note; childNote: Note; differentChildNote: Note; grandChildNote: Note; } = { user: {} as User, + user2: {} as User, parentNote: {} as Note, childNote: {} as Note, differentChildNote: {} as Note, @@ -199,7 +201,7 @@ describe('Note API', () => { /** Create test user */ context.user = await global.db.insertUser(); - const user2 = await global.db.insertUser(); + context.user2 = await global.db.insertUser(); /** Create test note - a parent note */ context.parentNote = await global.db.insertNote({ @@ -213,7 +215,7 @@ describe('Note API', () => { /** Create test note - create note with different user */ context.differentChildNote = await global.db.insertNote({ - creatorId: user2.id, + creatorId: context.user2.id, }); /** Create test note - a grandchild note */ @@ -564,48 +566,75 @@ describe('Note API', () => { test.each([ /** Returns two parents in case of relation between child and parent notes with 200 status */ { + testCase: 1, numberOfNotes: 2, - userNoteCreationDifferent: false, + childNoteCreatedByDifferentUser: false, isPublic: true, + expectedStatusCode: 200, }, /** Returns multiple parents in case of multiple notes relations with user presence in team in each note with 200 status */ { + testCase: 2, numberOfNotes: 3, - userNoteCreationDifferent: false, + childNoteCreatedByDifferentUser: false, isPublic: true, + expectedStatusCode: 200, }, /** Returns one parent in case where there is no note relation with 200 status */ { + testCase: 3, numberOfNotes: 1, - userNoteCreationDifferent: false, + childNoteCreatedByDifferentUser: false, isPublic: true, + expectedStatusCode: 200, }, /** Returns mutiple parents in case where user is not in the team of a note with 200 status */ { + testCase: 4, numberOfNotes: 3, - userNoteCreationDifferent: true, + childNoteCreatedByDifferentUser: true, isPublic: true, + expectedStatusCode: 200, }, /** Returns multiple parents in case when note is not public with 200 status */ { + testCase: 5, numberOfNotes: 3, - userNoteCreationDifferent: true, + childNoteCreatedByDifferentUser: true, isPublic: false, + expectedStatusCode: 200, }, - ])('Get note parents in different scenarios', async ({ numberOfNotes, userNoteCreationDifferent, isPublic }) => { - /** The variable that stores the expected result */ - let expectedOutput: { id: string; content: object }[] = []; - + /** Returns no note in case when the user is not authorized to note with 403 status */ + { + testCase: 6, + numberOfNotes: 2, + childNoteCreatedByDifferentUser: true, + isPublic: false, + expectedStatusCode: 403, + }, + /** Returns one note in case when the user has no access to note with 200 status */ + { + testCase: 7, + numberOfNotes: 2, + childNoteCreatedByDifferentUser: true, + isPublic: true, + expectedStatusCode: 200, + }, + ])('Get note parents in different scenarios', async ({ testCase, numberOfNotes, childNoteCreatedByDifferentUser, isPublic, expectedStatusCode }) => { if (context !== undefined) { /** Create acces token for the user */ - const accessToken = global.auth(context.user.id); + let accessToken = global.auth(context.user.id); + + if (testCase === 6 || testCase == 7) { + accessToken = global.auth(context.user2.id); + } let noteId = context.parentNote.id; let notePublicId = context.parentNote.publicId; - if (numberOfNotes == 2 && !userNoteCreationDifferent) { + if (numberOfNotes == 2 && !childNoteCreatedByDifferentUser) { noteId = context.childNote.id; notePublicId = context.childNote.publicId; - } else if (numberOfNotes == 2 && userNoteCreationDifferent) { + } else if (numberOfNotes == 2 && childNoteCreatedByDifferentUser) { noteId = context.differentChildNote.id; notePublicId = context.differentChildNote.publicId; } else if (numberOfNotes == 3) { @@ -622,31 +651,11 @@ describe('Note API', () => { for (let i = 0; i < numberOfNotes - 1; i++) { /** Create test note relation */ await global.db.insertNoteRelation({ - parentId: i == 0 ? context.parentNote.id : (userNoteCreationDifferent ? context.differentChildNote.id : context.childNote.id), - noteId: i == 0 ? (userNoteCreationDifferent ? context.differentChildNote.id : context.childNote.id) : context.grandChildNote.id, + parentId: i == 0 ? context.parentNote.id : (childNoteCreatedByDifferentUser ? context.differentChildNote.id : context.childNote.id), + noteId: i == 0 ? (childNoteCreatedByDifferentUser ? context.differentChildNote.id : context.childNote.id) : context.grandChildNote.id, }); } - /** The expected output that should be returned in the response */ - for (let i = 0; i < numberOfNotes; i++) { - if (i == 0) { - expectedOutput.push({ - id: context.parentNote.publicId, - content: context.parentNote.content, - }); - } else if (i == 1) { - expectedOutput.push({ - id: userNoteCreationDifferent ? context.differentChildNote.publicId : context.childNote.publicId, - content: userNoteCreationDifferent ? context.differentChildNote.content : context.childNote.content, - }); - } else { - expectedOutput.push({ - id: context.grandChildNote.publicId, - content: context.grandChildNote.content, - }); - } - } - const response = await global.api?.fakeRequest({ method: 'GET', headers: { @@ -655,11 +664,87 @@ describe('Note API', () => { url: `/note/${notePublicId}`, }); - expect(response?.statusCode).toBe(200); - - expect(response?.json()).toMatchObject({ - parents: expectedOutput, - }); + switch (testCase) { + case (1): + expect(response?.statusCode).toBe(expectedStatusCode); + expect(response?.json()).toMatchObject({ + parents: [ + { + id: context.parentNote.publicId, + content: context.parentNote.content, + }, + { + id: context.childNote.publicId, + content: context.childNote.content, + }, + ], + }); + break; + case (2): + expect(response?.statusCode).toBe(expectedStatusCode); + expect(response?.json()).toMatchObject({ + parents: [ + { + id: context.parentNote.publicId, + content: context.parentNote.content, + }, + { + id: context.childNote.publicId, + content: context.childNote.content, + }, + { + id: context.grandChildNote.publicId, + content: context.grandChildNote.content, + }, + ], + }); + break; + case (3): + expect(response?.statusCode).toBe(expectedStatusCode); + expect(response?.json()).toMatchObject({ + parents: [ + { + id: context.parentNote.publicId, + content: context.parentNote.content, + }, + ], + }); + break; + case (4): + case (5): + expect(response?.statusCode).toBe(expectedStatusCode); + expect(response?.json()).toMatchObject({ + parents: [ + { + id: context.parentNote.publicId, + content: context.parentNote.content, + }, + { + id: context.differentChildNote.publicId, + content: context.differentChildNote.content, + }, + { + id: context.grandChildNote.publicId, + content: context.grandChildNote.content, + }, + ], + }); + break; + case (6): + expect(response?.statusCode).toBe(expectedStatusCode); + break; + case (7): + expect(response?.statusCode).toBe(expectedStatusCode); + expect(response?.json()).toMatchObject({ + parents: [ + { + id: context.differentChildNote.publicId, + content: context.differentChildNote.content, + }, + ], + }); + break; + } } }); }); From 5c6cfc0f8c93233759b4e01d3bf97fb646ff256e Mon Sep 17 00:00:00 2001 From: dependentmadani Date: Mon, 23 Sep 2024 20:44:05 +0100 Subject: [PATCH 16/31] update (parent notes): few modification done, still work to be done --- src/domain/service/note.ts | 15 +++- src/presentation/http/router/note.test.ts | 80 ++++++++++--------- src/presentation/http/router/note.ts | 2 +- src/repository/index.ts | 1 - src/repository/note.repository.ts | 8 +- .../storage/postgres/orm/sequelize/note.ts | 67 +++------------- 6 files changed, 67 insertions(+), 106 deletions(-) diff --git a/src/domain/service/note.ts b/src/domain/service/note.ts index 7571169d..e9531889 100644 --- a/src/domain/service/note.ts +++ b/src/domain/service/note.ts @@ -447,10 +447,19 @@ export default class NoteService { * Get note parent structure recursively by note id and user id * and check if user has access to the parent note. * @param noteId - id of the note to get parent structure - * @param userId - id of the user that is requesting the parent structure * @returns - array of notes that are parent structure of the note */ - public async getNoteParentStructure(noteId: NoteInternalId, userId: number): Promise { - return await this.noteRepository.getNoteParents(noteId, userId); + public async getNoteParents(noteId: NoteInternalId): Promise { + const noteParents = await this.noteRepository.getNoteParents(noteId); + const noteParentsPublic: NotePublic[] = noteParents.map((note) => { + console.log('note content inside map:', note); + + return { + ...note, + id: note.publicId, + }; + }); + + return noteParentsPublic; } } diff --git a/src/presentation/http/router/note.test.ts b/src/presentation/http/router/note.test.ts index 1394babb..840180ab 100644 --- a/src/presentation/http/router/note.test.ts +++ b/src/presentation/http/router/note.test.ts @@ -183,14 +183,14 @@ describe('Note API', () => { describe('GET note/:notePublicId ', () => { let context: { user: User; - user2: User; + anotherUser: User; parentNote: Note; childNote: Note; differentChildNote: Note; grandChildNote: Note; } = { user: {} as User, - user2: {} as User, + anotherUser: {} as User, parentNote: {} as Note, childNote: {} as Note, differentChildNote: {} as Note, @@ -198,30 +198,32 @@ describe('Note API', () => { }; beforeEach(async () => { - /** Create test user */ - context.user = await global.db.insertUser(); + if (expect.getState().currentTestName === 'Returns note parents by note public id in different note accessibility and authorization') { + /** Create test user */ + context.user = await global.db.insertUser(); - context.user2 = await global.db.insertUser(); + context.anotherUser = await global.db.insertUser(); - /** Create test note - a parent note */ - context.parentNote = await global.db.insertNote({ - creatorId: context.user.id, - }); + /** Create test note - a parent note */ + context.parentNote = await global.db.insertNote({ + creatorId: context.user.id, + }); - /** Create test note - a child note */ - context.childNote = await global.db.insertNote({ - creatorId: context.user.id, - }); + /** Create test note - a child note */ + context.childNote = await global.db.insertNote({ + creatorId: context.user.id, + }); - /** Create test note - create note with different user */ - context.differentChildNote = await global.db.insertNote({ - creatorId: context.user2.id, - }); + /** Create test note - create note with different user */ + context.differentChildNote = await global.db.insertNote({ + creatorId: context.anotherUser.id, + }); - /** Create test note - a grandchild note */ - context.grandChildNote = await global.db.insertNote({ - creatorId: context.user.id, - }); + /** Create test note - a grandchild note */ + context.grandChildNote = await global.db.insertNote({ + creatorId: context.user.id, + }); + } }); test.each([ @@ -564,69 +566,69 @@ describe('Note API', () => { }); test.each([ - /** Returns two parents in case of relation between child and parent notes with 200 status */ + /** Returns two parents in case of relation between child and parent notes */ { - testCase: 1, + testScenario: 1, numberOfNotes: 2, childNoteCreatedByDifferentUser: false, isPublic: true, expectedStatusCode: 200, }, - /** Returns multiple parents in case of multiple notes relations with user presence in team in each note with 200 status */ + /** Returns multiple parents in case of multiple notes relations with user presence in team in each note */ { - testCase: 2, + testScenario: 2, numberOfNotes: 3, childNoteCreatedByDifferentUser: false, isPublic: true, expectedStatusCode: 200, }, - /** Returns one parent in case where there is no note relation with 200 status */ + /** Returns one parent in case where there is no note relation */ { - testCase: 3, + testScenario: 3, numberOfNotes: 1, childNoteCreatedByDifferentUser: false, isPublic: true, expectedStatusCode: 200, }, - /** Returns mutiple parents in case where user is not in the team of a note with 200 status */ + /** Returns mutiple parents in case where user is not in the team of a note */ { - testCase: 4, + testScenario: 4, numberOfNotes: 3, childNoteCreatedByDifferentUser: true, isPublic: true, expectedStatusCode: 200, }, - /** Returns multiple parents in case when note is not public with 200 status */ + /** Returns multiple parents in case when note is not public */ { - testCase: 5, + testScenario: 5, numberOfNotes: 3, childNoteCreatedByDifferentUser: true, isPublic: false, expectedStatusCode: 200, }, - /** Returns no note in case when the user is not authorized to note with 403 status */ + /** Returns no note in case when the user is not authorized to note */ { - testCase: 6, + testScenario: 6, numberOfNotes: 2, childNoteCreatedByDifferentUser: true, isPublic: false, expectedStatusCode: 403, }, - /** Returns one note in case when the user has no access to note with 200 status */ + /** Returns one note in case when the user has no access to note */ { - testCase: 7, + testScenario: 7, numberOfNotes: 2, childNoteCreatedByDifferentUser: true, isPublic: true, expectedStatusCode: 200, }, - ])('Get note parents in different scenarios', async ({ testCase, numberOfNotes, childNoteCreatedByDifferentUser, isPublic, expectedStatusCode }) => { + ])('Returns note parents by note public id in different note accessibility and authorization', async ({ testScenario, numberOfNotes, childNoteCreatedByDifferentUser, isPublic, expectedStatusCode }) => { if (context !== undefined) { /** Create acces token for the user */ let accessToken = global.auth(context.user.id); - if (testCase === 6 || testCase == 7) { - accessToken = global.auth(context.user2.id); + if (testScenario === 6 || testScenario == 7) { + accessToken = global.auth(context.anotherUser.id); } let noteId = context.parentNote.id; let notePublicId = context.parentNote.publicId; @@ -664,7 +666,7 @@ describe('Note API', () => { url: `/note/${notePublicId}`, }); - switch (testCase) { + switch (testScenario) { case (1): expect(response?.statusCode).toBe(expectedStatusCode); expect(response?.json()).toMatchObject({ diff --git a/src/presentation/http/router/note.ts b/src/presentation/http/router/note.ts index fa00fe65..1d2a715d 100644 --- a/src/presentation/http/router/note.ts +++ b/src/presentation/http/router/note.ts @@ -179,7 +179,7 @@ const NoteRouter: FastifyPluginCallback = (fastify, opts, don */ const canEdit = memberRole === MemberRole.Write; - const noteParentStructure = await noteService.getNoteParentStructure(noteId, userId!); + const noteParentStructure = await noteService.getNoteParents(noteId); return reply.send({ note: notePublic, diff --git a/src/repository/index.ts b/src/repository/index.ts index d21da8ec..7963511f 100644 --- a/src/repository/index.ts +++ b/src/repository/index.ts @@ -133,7 +133,6 @@ export async function init(orm: Orm, s3Config: S3StorageConfig): Promise { - return await this.storage.getAllNoteParents(noteId, userId); + public async getNoteParents(noteId: NoteInternalId): Promise { + return await this.storage.getAllNoteParents(noteId); } } diff --git a/src/repository/storage/postgres/orm/sequelize/note.ts b/src/repository/storage/postgres/orm/sequelize/note.ts index 200b6b70..b67227e1 100644 --- a/src/repository/storage/postgres/orm/sequelize/note.ts +++ b/src/repository/storage/postgres/orm/sequelize/note.ts @@ -8,8 +8,6 @@ import type { NoteVisitsModel } from './noteVisits.js'; import type { NoteHistoryModel } from './noteHistory.js'; import type { NoteRelationsModel } from './noteRelations.js'; import { notEmpty } from '@infrastructure/utils/empty.js'; -import type { TeamsModel } from './teams.js'; -import type { NotePublic } from '@domain/entities/notePublic.js'; /* eslint-disable @typescript-eslint/naming-convention */ @@ -81,8 +79,6 @@ export default class NoteSequelizeStorage { public noteRelationModel: typeof NoteRelationsModel | null = null; - public teamModel: typeof TeamsModel | null = null; - /** * Database instance */ @@ -170,13 +166,6 @@ export default class NoteSequelizeStorage { this.noteRelationModel = model; } - public createAssociationWithTeamsModel(model: ModelStatic): void { - /** - * Create association with teams - */ - this.teamModel = model; - } - /** * Insert note to database * @param options - note creation options @@ -348,61 +337,25 @@ export default class NoteSequelizeStorage { /** * Get all parent notes of a note that a user has access to, - * by checking the team access. + * where the user has access to. * @param noteId - the ID of the note. - * @param userId - the ID of the user. */ - public async getAllNoteParents(noteId: NoteInternalId, userId: number): Promise { - if (!this.teamModel || !this.noteRelationModel) { - throw new Error(`${this.model !== null ? 'TeamStorage: Note relation model is not defined' : 'TeamStorage: Note model is not defined'}`); + public async getAllNoteParents(noteId: NoteInternalId): Promise { + if (!this.noteRelationModel) { + throw new Error('NoteStorage: Note Relation model is not defined'); } - const parentNotes: NotePublic[] = []; + const parentNotes: Note[] = []; let currentNoteId: NoteInternalId | null = noteId; - /** - * Store notes that user can not access, to check the inherited team if has access - */ - let storeUnaccessibleNote: Note[] = []; while (currentNoteId != null) { - const teamMember = await this.teamModel.findOne({ - where: { - noteId: currentNoteId, - userId, - }, - include: { - model: this.model, - as: this.model.tableName, - required: true, - }, + // Get the note for database + const note: Note | null = await this.model.findOne({ + where: { id: currentNoteId }, }); - if (teamMember && notEmpty(teamMember.notes)) { - if (storeUnaccessibleNote.length > 0) { - parentNotes.push(...storeUnaccessibleNote.map(note => ({ - id: note.publicId, - content: note.content, - updatedAt: note.updatedAt, - createdAt: note.createdAt, - creatorId: note.creatorId, - }))); - storeUnaccessibleNote = []; - } - parentNotes.push({ - id: teamMember.notes.publicId, - content: teamMember.notes.content, - updatedAt: teamMember.notes.updatedAt, - createdAt: teamMember.notes.createdAt, - creatorId: teamMember.notes.creatorId, - }); - } else if (teamMember === null) { - const note = await this.model.findOne({ - where: { id: currentNoteId }, - }); - - if (note !== null) { - storeUnaccessibleNote.push(note); - } + if (notEmpty(note)) { + parentNotes.push(note); } // Retrieve the parent note From 5bb6be5cacba7e7920711498ee4b5702e9c35f46 Mon Sep 17 00:00:00 2001 From: dependentmadani Date: Tue, 24 Sep 2024 18:46:10 +0100 Subject: [PATCH 17/31] update (note parents): fix the issue of tests, working good --- src/domain/service/note.ts | 7 ++++--- src/presentation/http/router/note.test.ts | 6 +++++- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/src/domain/service/note.ts b/src/domain/service/note.ts index e9531889..c6eddb47 100644 --- a/src/domain/service/note.ts +++ b/src/domain/service/note.ts @@ -452,11 +452,12 @@ export default class NoteService { public async getNoteParents(noteId: NoteInternalId): Promise { const noteParents = await this.noteRepository.getNoteParents(noteId); const noteParentsPublic: NotePublic[] = noteParents.map((note) => { - console.log('note content inside map:', note); - return { - ...note, + content: note.content, id: note.publicId, + creatorId: note.creatorId, + createdAt: note.createdAt, + updatedAt: note.updatedAt, }; }); diff --git a/src/presentation/http/router/note.test.ts b/src/presentation/http/router/note.test.ts index 840180ab..9842a7b5 100644 --- a/src/presentation/http/router/note.test.ts +++ b/src/presentation/http/router/note.test.ts @@ -198,7 +198,7 @@ describe('Note API', () => { }; beforeEach(async () => { - if (expect.getState().currentTestName === 'Returns note parents by note public id in different note accessibility and authorization') { + if (expect.getState().currentTestName?.includes('Returns note parents by note public id in different note accessibility and authorization') ?? false) { /** Create test user */ context.user = await global.db.insertUser(); @@ -739,6 +739,10 @@ describe('Note API', () => { expect(response?.statusCode).toBe(expectedStatusCode); expect(response?.json()).toMatchObject({ parents: [ + { + id: context.parentNote.publicId, + content: context.parentNote.content, + }, { id: context.differentChildNote.publicId, content: context.differentChildNote.content, From fee9bbf0480cf71f36d31656676b542cf0448063 Mon Sep 17 00:00:00 2001 From: dependentmadani Date: Thu, 26 Sep 2024 13:17:02 +0100 Subject: [PATCH 18/31] update (note parents): modification based on previous review, chore modification of logic --- src/domain/service/note.ts | 13 +- src/presentation/http/router/note.test.ts | 386 ++++++++++-------- src/repository/note.repository.ts | 9 - src/repository/noteRelations.repository.ts | 20 +- .../storage/postgres/orm/sequelize/note.ts | 41 -- .../postgres/orm/sequelize/noteRelations.ts | 65 +++ 6 files changed, 294 insertions(+), 240 deletions(-) diff --git a/src/domain/service/note.ts b/src/domain/service/note.ts index c6eddb47..fc2a464b 100644 --- a/src/domain/service/note.ts +++ b/src/domain/service/note.ts @@ -9,7 +9,7 @@ import type User from '@domain/entities/user.js'; import type { NoteList } from '@domain/entities/noteList.js'; import type NoteHistoryRepository from '@repository/noteHistory.repository.js'; import type { NoteHistoryMeta, NoteHistoryRecord, NoteHistoryPublic } from '@domain/entities/noteHistory.js'; -import type { NotePublic } from '@domain/entities/notePublic.js'; +import { definePublicNote, type NotePublic } from '@domain/entities/notePublic.js'; /** * Note service @@ -450,15 +450,10 @@ export default class NoteService { * @returns - array of notes that are parent structure of the note */ public async getNoteParents(noteId: NoteInternalId): Promise { - const noteParents = await this.noteRepository.getNoteParents(noteId); + const noteIds: NoteInternalId[] = await this.noteRelationsRepository.getNoteParentsIds(noteId); + const noteParents = await this.noteRelationsRepository.getNotesByIds(noteIds); const noteParentsPublic: NotePublic[] = noteParents.map((note) => { - return { - content: note.content, - id: note.publicId, - creatorId: note.creatorId, - createdAt: note.createdAt, - updatedAt: note.updatedAt, - }; + return definePublicNote(note); }); return noteParentsPublic; diff --git a/src/presentation/http/router/note.test.ts b/src/presentation/http/router/note.test.ts index 9842a7b5..1a794b78 100644 --- a/src/presentation/http/router/note.test.ts +++ b/src/presentation/http/router/note.test.ts @@ -565,193 +565,219 @@ describe('Note API', () => { expect(response?.json().message).toStrictEqual(expectedMessage); }); - test.each([ - /** Returns two parents in case of relation between child and parent notes */ - { - testScenario: 1, - numberOfNotes: 2, - childNoteCreatedByDifferentUser: false, - isPublic: true, - expectedStatusCode: 200, - }, - /** Returns multiple parents in case of multiple notes relations with user presence in team in each note */ - { - testScenario: 2, - numberOfNotes: 3, - childNoteCreatedByDifferentUser: false, - isPublic: true, - expectedStatusCode: 200, - }, - /** Returns one parent in case where there is no note relation */ - { - testScenario: 3, - numberOfNotes: 1, - childNoteCreatedByDifferentUser: false, + test('Returns two parents note in case of relation between child and parent note with status 200', async () => { + /** Create test user */ + const user = await global.db.insertUser(); + + /** Create acces token for the user */ + const accessToken = global.auth(user.id); + + /** Create test note - a parent note */ + const parentNote = await global.db.insertNote({ + creatorId: user.id, + }); + + /** Create test note - a child note */ + const childNote = await global.db.insertNote({ + creatorId: user.id, + }); + + /** Create test note settings */ + await global.db.insertNoteSetting({ + noteId: childNote.id, isPublic: true, - expectedStatusCode: 200, - }, - /** Returns mutiple parents in case where user is not in the team of a note */ - { - testScenario: 4, - numberOfNotes: 3, - childNoteCreatedByDifferentUser: true, + }); + + /** Create test note relation */ + await global.db.insertNoteRelation({ + parentId: parentNote.id, + noteId: childNote.id, + }); + + const response = await global.api?.fakeRequest({ + method: 'GET', + headers: { + authorization: `Bearer ${accessToken}`, + }, + url: `/note/${childNote.publicId}`, + }); + + expect(response?.statusCode).toBe(200); + + expect(response?.json()).toMatchObject({ + parentStructure: [ + { + noteId: parentNote.publicId, + content: parentNote.content, + }, + { + noteId: childNote.publicId, + content: childNote.content, + }, + ], + }); + }); + + test('Returns three parents note in case of relation between all notes with status 200', async () => { + /** Create test user */ + const user = await global.db.insertUser(); + + /** Create acces token for the user */ + const accessToken = global.auth(user.id); + + /** Create test note - a grand parent note */ + const grandParentNote = await global.db.insertNote({ + creatorId: user.id, + }); + + /** Create test note - a parent note */ + const parentNote = await global.db.insertNote({ + creatorId: user.id, + }); + + /** Create test note - a child note */ + const childNote = await global.db.insertNote({ + creatorId: user.id, + }); + + /** Create test note settings */ + await global.db.insertNoteSetting({ + noteId: childNote.id, isPublic: true, - expectedStatusCode: 200, - }, - /** Returns multiple parents in case when note is not public */ - { - testScenario: 5, - numberOfNotes: 3, - childNoteCreatedByDifferentUser: true, - isPublic: false, - expectedStatusCode: 200, - }, - /** Returns no note in case when the user is not authorized to note */ - { - testScenario: 6, - numberOfNotes: 2, - childNoteCreatedByDifferentUser: true, - isPublic: false, - expectedStatusCode: 403, - }, - /** Returns one note in case when the user has no access to note */ - { - testScenario: 7, - numberOfNotes: 2, - childNoteCreatedByDifferentUser: true, + }); + + /** Create note relation between parent and grandParentNote */ + await global.db.insertNoteRelation({ + parentId: grandParentNote.id, + noteId: parentNote.id, + }); + + /** Create test note relation */ + await global.db.insertNoteRelation({ + parentId: parentNote.id, + noteId: childNote.id, + }); + + const response = await global.api?.fakeRequest({ + method: 'GET', + headers: { + authorization: `Bearer ${accessToken}`, + }, + url: `/note/${childNote.publicId}`, + }); + + expect(response?.statusCode).toBe(200); + + expect(response?.json()).toMatchObject({ + parentStructure: [ + { + noteId: grandParentNote.publicId, + content: grandParentNote.content, + }, + { + noteId: parentNote.publicId, + content: parentNote.content, + }, + { + noteId: childNote.publicId, + content: childNote.content, + }, + ], + }); + }); + + test('Returns two parents note in case where the note is not created by the same user but share relation with status 200', async () => { + /** Create test user */ + const user = await global.db.insertUser(); + + /** Create another user */ + const anotherUser = await global.db.insertUser(); + + /** Create acces token for the user */ + const accessToken = global.auth(user.id); + + /** Create test note - a parent note */ + const parentNote = await global.db.insertNote({ + creatorId: anotherUser.id, + }); + + /** Create test note - a child note */ + const childNote = await global.db.insertNote({ + creatorId: user.id, + }); + + /** Create test note settings */ + await global.db.insertNoteSetting({ + noteId: childNote.id, isPublic: true, - expectedStatusCode: 200, - }, - ])('Returns note parents by note public id in different note accessibility and authorization', async ({ testScenario, numberOfNotes, childNoteCreatedByDifferentUser, isPublic, expectedStatusCode }) => { - if (context !== undefined) { - /** Create acces token for the user */ - let accessToken = global.auth(context.user.id); - - if (testScenario === 6 || testScenario == 7) { - accessToken = global.auth(context.anotherUser.id); - } - let noteId = context.parentNote.id; - let notePublicId = context.parentNote.publicId; - - if (numberOfNotes == 2 && !childNoteCreatedByDifferentUser) { - noteId = context.childNote.id; - notePublicId = context.childNote.publicId; - } else if (numberOfNotes == 2 && childNoteCreatedByDifferentUser) { - noteId = context.differentChildNote.id; - notePublicId = context.differentChildNote.publicId; - } else if (numberOfNotes == 3) { - noteId = context.grandChildNote.id; - notePublicId = context.grandChildNote.publicId; - } - - /** Create test note settings */ - await global.db.insertNoteSetting({ - noteId: noteId, - isPublic: isPublic, - }); + }); - for (let i = 0; i < numberOfNotes - 1; i++) { - /** Create test note relation */ - await global.db.insertNoteRelation({ - parentId: i == 0 ? context.parentNote.id : (childNoteCreatedByDifferentUser ? context.differentChildNote.id : context.childNote.id), - noteId: i == 0 ? (childNoteCreatedByDifferentUser ? context.differentChildNote.id : context.childNote.id) : context.grandChildNote.id, - }); - } + /** Create test note relation */ + await global.db.insertNoteRelation({ + parentId: parentNote.id, + noteId: childNote.id, + }); - const response = await global.api?.fakeRequest({ - method: 'GET', - headers: { - authorization: `Bearer ${accessToken}`, + const response = await global.api?.fakeRequest({ + method: 'GET', + headers: { + authorization: `Bearer ${accessToken}`, + }, + url: `/note/${childNote.publicId}`, + }); + + expect(response?.statusCode).toBe(200); + + expect(response?.json()).toMatchObject({ + parentStructure: [ + { + noteId: parentNote.publicId, + content: parentNote.content, }, - url: `/note/${notePublicId}`, - }); + { + noteId: childNote.publicId, + content: childNote.content, + }, + ], + }); + }); - switch (testScenario) { - case (1): - expect(response?.statusCode).toBe(expectedStatusCode); - expect(response?.json()).toMatchObject({ - parents: [ - { - id: context.parentNote.publicId, - content: context.parentNote.content, - }, - { - id: context.childNote.publicId, - content: context.childNote.content, - }, - ], - }); - break; - case (2): - expect(response?.statusCode).toBe(expectedStatusCode); - expect(response?.json()).toMatchObject({ - parents: [ - { - id: context.parentNote.publicId, - content: context.parentNote.content, - }, - { - id: context.childNote.publicId, - content: context.childNote.content, - }, - { - id: context.grandChildNote.publicId, - content: context.grandChildNote.content, - }, - ], - }); - break; - case (3): - expect(response?.statusCode).toBe(expectedStatusCode); - expect(response?.json()).toMatchObject({ - parents: [ - { - id: context.parentNote.publicId, - content: context.parentNote.content, - }, - ], - }); - break; - case (4): - case (5): - expect(response?.statusCode).toBe(expectedStatusCode); - expect(response?.json()).toMatchObject({ - parents: [ - { - id: context.parentNote.publicId, - content: context.parentNote.content, - }, - { - id: context.differentChildNote.publicId, - content: context.differentChildNote.content, - }, - { - id: context.grandChildNote.publicId, - content: context.grandChildNote.content, - }, - ], - }); - break; - case (6): - expect(response?.statusCode).toBe(expectedStatusCode); - break; - case (7): - expect(response?.statusCode).toBe(expectedStatusCode); - expect(response?.json()).toMatchObject({ - parents: [ - { - id: context.parentNote.publicId, - content: context.parentNote.content, - }, - { - id: context.differentChildNote.publicId, - content: context.differentChildNote.content, - }, - ], - }); - break; - } - } + test('Returns one note in case where there is no relation exist for the note with status 200', async () => { + /** Create test user */ + const user = await global.db.insertUser(); + + /** Create acces token for the user */ + const accessToken = global.auth(user.id); + + /** Create test note - a child note */ + const note = await global.db.insertNote({ + creatorId: user.id, + }); + + /** Create test note settings */ + await global.db.insertNoteSetting({ + noteId: note.id, + isPublic: true, + }); + + const response = await global.api?.fakeRequest({ + method: 'GET', + headers: { + authorization: `Bearer ${accessToken}`, + }, + url: `/note/${note.publicId}`, + }); + + expect(response?.statusCode).toBe(200); + + expect(response?.json()).toMatchObject({ + parentStructure: [ + { + noteId: note.publicId, + content: note.content, + }, + ], + }); }); }); diff --git a/src/repository/note.repository.ts b/src/repository/note.repository.ts index eda11a92..0771f8b9 100644 --- a/src/repository/note.repository.ts +++ b/src/repository/note.repository.ts @@ -81,13 +81,4 @@ export default class NoteRepository { public async getNoteListByUserId(id: number, offset: number, limit: number): Promise { return await this.storage.getNoteListByUserId(id, offset, limit); } - - /** - * Get all note parents based on note id - * @param noteId : note id to get all its parents - * @returns an array of note parents objects containing public id and content - */ - public async getNoteParents(noteId: NoteInternalId): Promise { - return await this.storage.getAllNoteParents(noteId); - } } diff --git a/src/repository/noteRelations.repository.ts b/src/repository/noteRelations.repository.ts index f31c1a4e..9098eee8 100644 --- a/src/repository/noteRelations.repository.ts +++ b/src/repository/noteRelations.repository.ts @@ -1,4 +1,4 @@ -import type { NoteInternalId } from '@domain/entities/note.js'; +import type { Note, NoteInternalId } from '@domain/entities/note.js'; import type NoteRelationshipStorage from '@repository/storage/noteRelations.storage.js'; /** @@ -67,4 +67,22 @@ export default class NoteRelationsRepository { public async hasRelation(noteId: NoteInternalId): Promise { return await this.storage.hasRelation(noteId); } + + /** + * Get all note parents based on note id + * @param noteId : note id to get all its parents + * @returns an array of note parents ids + */ + public async getNoteParentsIds(noteId: NoteInternalId): Promise { + return await this.storage.getAllNoteParentsIds(noteId); + } + + /** + * Get all notes based on their ids + * @param noteIds : list of note ids + * @returns an array of notes + */ + public async getNotesByIds(noteIds: NoteInternalId[]): Promise { + return await this.storage.getNotesByIds(noteIds); + } } diff --git a/src/repository/storage/postgres/orm/sequelize/note.ts b/src/repository/storage/postgres/orm/sequelize/note.ts index b67227e1..7148f269 100644 --- a/src/repository/storage/postgres/orm/sequelize/note.ts +++ b/src/repository/storage/postgres/orm/sequelize/note.ts @@ -7,7 +7,6 @@ import type { NoteSettingsModel } from './noteSettings.js'; import type { NoteVisitsModel } from './noteVisits.js'; import type { NoteHistoryModel } from './noteHistory.js'; import type { NoteRelationsModel } from './noteRelations.js'; -import { notEmpty } from '@infrastructure/utils/empty.js'; /* eslint-disable @typescript-eslint/naming-convention */ @@ -334,44 +333,4 @@ export default class NoteSequelizeStorage { }, }); }; - - /** - * Get all parent notes of a note that a user has access to, - * where the user has access to. - * @param noteId - the ID of the note. - */ - public async getAllNoteParents(noteId: NoteInternalId): Promise { - if (!this.noteRelationModel) { - throw new Error('NoteStorage: Note Relation model is not defined'); - } - - const parentNotes: Note[] = []; - let currentNoteId: NoteInternalId | null = noteId; - - while (currentNoteId != null) { - // Get the note for database - const note: Note | null = await this.model.findOne({ - where: { id: currentNoteId }, - }); - - if (notEmpty(note)) { - parentNotes.push(note); - } - - // Retrieve the parent note - const noteRelation: NoteRelationsModel | null = await this.noteRelationModel.findOne({ - where: { noteId: currentNoteId }, - }); - - if (noteRelation != null) { - currentNoteId = noteRelation.parentId; - } else { - currentNoteId = null; - } - } - - parentNotes.reverse(); - - return parentNotes; - } } diff --git a/src/repository/storage/postgres/orm/sequelize/noteRelations.ts b/src/repository/storage/postgres/orm/sequelize/noteRelations.ts index 879ddb98..c0dee786 100644 --- a/src/repository/storage/postgres/orm/sequelize/noteRelations.ts +++ b/src/repository/storage/postgres/orm/sequelize/noteRelations.ts @@ -5,6 +5,7 @@ import type Orm from '@repository/storage/postgres/orm/sequelize/index.js'; import type { NoteInternalId } from '@domain/entities/note.js'; import type { Note } from '@domain/entities/note.js'; import { Model, DataTypes } from 'sequelize'; +import { notEmpty } from '@infrastructure/utils/empty.js'; /** * Class representing a note relations in database @@ -209,4 +210,68 @@ export default class NoteRelationsSequelizeStorage { return foundNote !== null; }; + + /** + * Get all parent notes of a note that a user has access to, + * where the user has access to. + * @param noteId - the ID of the note. + */ + public async getAllNoteParentsIds(noteId: NoteInternalId): Promise { + if (!this.noteModel) { + throw new Error('NoteRelationStorage: Note model is not defined'); + } + + const parentNotes: NoteInternalId[] = []; + let currentNoteId: NoteInternalId | null = noteId; + + while (currentNoteId != null) { + // Get the note for database + const note: Note | null = await this.noteModel.findOne({ + where: { id: currentNoteId }, + }); + + if (notEmpty(note)) { + parentNotes.push(note.id); + } + + // Retrieve the parent note + const noteRelation: NoteRelationsModel | null = await this.model.findOne({ + where: { noteId: currentNoteId }, + }); + + if (noteRelation != null) { + currentNoteId = noteRelation.parentId; + } else { + currentNoteId = null; + } + } + + parentNotes.reverse(); + + return parentNotes; + } + + /** + * Get all notes based on their ids + * @param noteIds - list of note ids + */ + public async getNotesByIds(noteIds: NoteInternalId[]): Promise { + if (!this.noteModel) { + throw new Error('NoteRelationStorage: Note model is not defined'); + } + + const notes: Note[] = []; + + for (const noteId of noteIds) { + const note: Note | null = await this.noteModel.findOne({ + where: { id: noteId }, + }); + + if (notEmpty(note)) { + notes.push(note); + } + } + + return notes; + } } From d65c5891a920c023a1f5dca5bd1ddaf256e1380e Mon Sep 17 00:00:00 2001 From: dependentmadani Date: Thu, 26 Sep 2024 19:55:34 +0100 Subject: [PATCH 19/31] update (note parents tests): the issue of test failure is fixed --- src/presentation/http/router/note.test.ts | 24 +++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/src/presentation/http/router/note.test.ts b/src/presentation/http/router/note.test.ts index 1a794b78..ae2656b6 100644 --- a/src/presentation/http/router/note.test.ts +++ b/src/presentation/http/router/note.test.ts @@ -605,13 +605,13 @@ describe('Note API', () => { expect(response?.statusCode).toBe(200); expect(response?.json()).toMatchObject({ - parentStructure: [ + parents: [ { - noteId: parentNote.publicId, + id: parentNote.publicId, content: parentNote.content, }, { - noteId: childNote.publicId, + id: childNote.publicId, content: childNote.content, }, ], @@ -669,17 +669,17 @@ describe('Note API', () => { expect(response?.statusCode).toBe(200); expect(response?.json()).toMatchObject({ - parentStructure: [ + parents: [ { - noteId: grandParentNote.publicId, + id: grandParentNote.publicId, content: grandParentNote.content, }, { - noteId: parentNote.publicId, + id: parentNote.publicId, content: parentNote.content, }, { - noteId: childNote.publicId, + id: childNote.publicId, content: childNote.content, }, ], @@ -729,13 +729,13 @@ describe('Note API', () => { expect(response?.statusCode).toBe(200); expect(response?.json()).toMatchObject({ - parentStructure: [ + parents: [ { - noteId: parentNote.publicId, + id: parentNote.publicId, content: parentNote.content, }, { - noteId: childNote.publicId, + id: childNote.publicId, content: childNote.content, }, ], @@ -771,9 +771,9 @@ describe('Note API', () => { expect(response?.statusCode).toBe(200); expect(response?.json()).toMatchObject({ - parentStructure: [ + parents: [ { - noteId: note.publicId, + id: note.publicId, content: note.content, }, ], From 14234a12ae6126cd8b383efac638ae8ce70a031d Mon Sep 17 00:00:00 2001 From: dependentmadani Date: Fri, 27 Sep 2024 19:18:20 +0100 Subject: [PATCH 20/31] udpate (note parents): based on last review --- src/domain/service/note.ts | 2 +- src/presentation/http/router/note.test.ts | 76 ++++--------------- src/repository/index.ts | 1 - src/repository/note.repository.ts | 9 +++ src/repository/noteRelations.repository.ts | 11 +-- .../storage/postgres/orm/sequelize/note.ts | 31 +++++--- .../postgres/orm/sequelize/noteRelations.ts | 52 ++----------- 7 files changed, 55 insertions(+), 127 deletions(-) diff --git a/src/domain/service/note.ts b/src/domain/service/note.ts index fc2a464b..42ae2b7b 100644 --- a/src/domain/service/note.ts +++ b/src/domain/service/note.ts @@ -451,7 +451,7 @@ export default class NoteService { */ public async getNoteParents(noteId: NoteInternalId): Promise { const noteIds: NoteInternalId[] = await this.noteRelationsRepository.getNoteParentsIds(noteId); - const noteParents = await this.noteRelationsRepository.getNotesByIds(noteIds); + const noteParents = await this.noteRepository.getNotesByIds(noteIds); const noteParentsPublic: NotePublic[] = noteParents.map((note) => { return definePublicNote(note); }); diff --git a/src/presentation/http/router/note.test.ts b/src/presentation/http/router/note.test.ts index 1a794b78..addfe180 100644 --- a/src/presentation/http/router/note.test.ts +++ b/src/presentation/http/router/note.test.ts @@ -1,7 +1,6 @@ import { MemberRole } from '@domain/entities/team.js'; import { describe, test, expect, beforeEach } from 'vitest'; import type User from '@domain/entities/user.js'; -import type { Note } from '@domain/entities/note.js'; describe('Note API', () => { /** @@ -181,51 +180,6 @@ describe('Note API', () => { }); describe('GET note/:notePublicId ', () => { - let context: { - user: User; - anotherUser: User; - parentNote: Note; - childNote: Note; - differentChildNote: Note; - grandChildNote: Note; - } = { - user: {} as User, - anotherUser: {} as User, - parentNote: {} as Note, - childNote: {} as Note, - differentChildNote: {} as Note, - grandChildNote: {} as Note, - }; - - beforeEach(async () => { - if (expect.getState().currentTestName?.includes('Returns note parents by note public id in different note accessibility and authorization') ?? false) { - /** Create test user */ - context.user = await global.db.insertUser(); - - context.anotherUser = await global.db.insertUser(); - - /** Create test note - a parent note */ - context.parentNote = await global.db.insertNote({ - creatorId: context.user.id, - }); - - /** Create test note - a child note */ - context.childNote = await global.db.insertNote({ - creatorId: context.user.id, - }); - - /** Create test note - create note with different user */ - context.differentChildNote = await global.db.insertNote({ - creatorId: context.anotherUser.id, - }); - - /** Create test note - a grandchild note */ - context.grandChildNote = await global.db.insertNote({ - creatorId: context.user.id, - }); - } - }); - test.each([ /** Returns 200 if user is team member with a Read role */ { @@ -565,7 +519,7 @@ describe('Note API', () => { expect(response?.json().message).toStrictEqual(expectedMessage); }); - test('Returns two parents note in case of relation between child and parent note with status 200', async () => { + test('Returns one parents note in case when note has one parents', async () => { /** Create test user */ const user = await global.db.insertUser(); @@ -605,20 +559,20 @@ describe('Note API', () => { expect(response?.statusCode).toBe(200); expect(response?.json()).toMatchObject({ - parentStructure: [ + parents: [ { - noteId: parentNote.publicId, + id: parentNote.publicId, content: parentNote.content, }, { - noteId: childNote.publicId, + id: childNote.publicId, content: childNote.content, }, ], }); }); - test('Returns three parents note in case of relation between all notes with status 200', async () => { + test('Returns two parents note in case when note has two parents', async () => { /** Create test user */ const user = await global.db.insertUser(); @@ -669,24 +623,24 @@ describe('Note API', () => { expect(response?.statusCode).toBe(200); expect(response?.json()).toMatchObject({ - parentStructure: [ + parents: [ { - noteId: grandParentNote.publicId, + id: grandParentNote.publicId, content: grandParentNote.content, }, { - noteId: parentNote.publicId, + id: parentNote.publicId, content: parentNote.content, }, { - noteId: childNote.publicId, + id: childNote.publicId, content: childNote.content, }, ], }); }); - test('Returns two parents note in case where the note is not created by the same user but share relation with status 200', async () => { + test('Returns one parents note in case where the note is not created by the same user but share relation', async () => { /** Create test user */ const user = await global.db.insertUser(); @@ -729,13 +683,13 @@ describe('Note API', () => { expect(response?.statusCode).toBe(200); expect(response?.json()).toMatchObject({ - parentStructure: [ + parents: [ { - noteId: parentNote.publicId, + id: parentNote.publicId, content: parentNote.content, }, { - noteId: childNote.publicId, + id: childNote.publicId, content: childNote.content, }, ], @@ -771,9 +725,9 @@ describe('Note API', () => { expect(response?.statusCode).toBe(200); expect(response?.json()).toMatchObject({ - parentStructure: [ + parents: [ { - noteId: note.publicId, + id: note.publicId, content: note.content, }, ], diff --git a/src/repository/index.ts b/src/repository/index.ts index 7963511f..ed40d01e 100644 --- a/src/repository/index.ts +++ b/src/repository/index.ts @@ -139,7 +139,6 @@ export async function init(orm: Orm, s3Config: S3StorageConfig): Promise { return await this.storage.getNoteListByUserId(id, offset, limit); } + + /** + * Get all notes based on their ids + * @param noteIds : list of note ids + * @returns an array of notes + */ + public async getNotesByIds(noteIds: NoteInternalId[]): Promise { + return await this.storage.getNotesByIds(noteIds); + } } diff --git a/src/repository/noteRelations.repository.ts b/src/repository/noteRelations.repository.ts index 9098eee8..038dc696 100644 --- a/src/repository/noteRelations.repository.ts +++ b/src/repository/noteRelations.repository.ts @@ -1,4 +1,4 @@ -import type { Note, NoteInternalId } from '@domain/entities/note.js'; +import type { NoteInternalId } from '@domain/entities/note.js'; import type NoteRelationshipStorage from '@repository/storage/noteRelations.storage.js'; /** @@ -76,13 +76,4 @@ export default class NoteRelationsRepository { public async getNoteParentsIds(noteId: NoteInternalId): Promise { return await this.storage.getAllNoteParentsIds(noteId); } - - /** - * Get all notes based on their ids - * @param noteIds : list of note ids - * @returns an array of notes - */ - public async getNotesByIds(noteIds: NoteInternalId[]): Promise { - return await this.storage.getNotesByIds(noteIds); - } } diff --git a/src/repository/storage/postgres/orm/sequelize/note.ts b/src/repository/storage/postgres/orm/sequelize/note.ts index 7148f269..d789130e 100644 --- a/src/repository/storage/postgres/orm/sequelize/note.ts +++ b/src/repository/storage/postgres/orm/sequelize/note.ts @@ -6,7 +6,7 @@ import { UserModel } from '@repository/storage/postgres/orm/sequelize/user.js'; import type { NoteSettingsModel } from './noteSettings.js'; import type { NoteVisitsModel } from './noteVisits.js'; import type { NoteHistoryModel } from './noteHistory.js'; -import type { NoteRelationsModel } from './noteRelations.js'; +import { notEmpty } from '@infrastructure/utils/empty.js'; /* eslint-disable @typescript-eslint/naming-convention */ @@ -76,8 +76,6 @@ export default class NoteSequelizeStorage { public historyModel: typeof NoteHistoryModel | null = null; - public noteRelationModel: typeof NoteRelationsModel | null = null; - /** * Database instance */ @@ -158,13 +156,6 @@ export default class NoteSequelizeStorage { }); }; - public createAssociationWithNoteRelationModel(model: ModelStatic): void { - /** - * Create association with note relations - */ - this.noteRelationModel = model; - } - /** * Insert note to database * @param options - note creation options @@ -333,4 +324,24 @@ export default class NoteSequelizeStorage { }, }); }; + + /** + * Get all notes based on their ids + * @param noteIds - list of note ids + */ + public async getNotesByIds(noteIds: NoteInternalId[]): Promise { + const notes: Note[] = []; + + for (const noteId of noteIds) { + const note: Note | null = await this.model.findOne({ + where: { id: noteId }, + }); + + if (notEmpty(note)) { + notes.push(note); + } + } + + return notes; + } } diff --git a/src/repository/storage/postgres/orm/sequelize/noteRelations.ts b/src/repository/storage/postgres/orm/sequelize/noteRelations.ts index c0dee786..e3154121 100644 --- a/src/repository/storage/postgres/orm/sequelize/noteRelations.ts +++ b/src/repository/storage/postgres/orm/sequelize/noteRelations.ts @@ -222,56 +222,20 @@ export default class NoteRelationsSequelizeStorage { } const parentNotes: NoteInternalId[] = []; - let currentNoteId: NoteInternalId | null = noteId; - while (currentNoteId != null) { - // Get the note for database - const note: Note | null = await this.noteModel.findOne({ - where: { id: currentNoteId }, - }); - - if (notEmpty(note)) { - parentNotes.push(note.id); - } - - // Retrieve the parent note - const noteRelation: NoteRelationsModel | null = await this.model.findOne({ - where: { noteId: currentNoteId }, - }); + // get all note ids via a singe sql query + const noteRelation: NoteRelationsModel[] | null = await this.model.findAll({ + where: { noteId }, + attributes: ['noteId'], + raw: true, + }); - if (noteRelation != null) { - currentNoteId = noteRelation.parentId; - } else { - currentNoteId = null; - } + if (notEmpty(noteRelation)) { + parentNotes.push(...noteRelation.map(relation => relation.noteId)); } parentNotes.reverse(); return parentNotes; } - - /** - * Get all notes based on their ids - * @param noteIds - list of note ids - */ - public async getNotesByIds(noteIds: NoteInternalId[]): Promise { - if (!this.noteModel) { - throw new Error('NoteRelationStorage: Note model is not defined'); - } - - const notes: Note[] = []; - - for (const noteId of noteIds) { - const note: Note | null = await this.noteModel.findOne({ - where: { id: noteId }, - }); - - if (notEmpty(note)) { - notes.push(note); - } - } - - return notes; - } } From 9872d5e3ab7b1dd339c2a67ff2e3d912d1e5b1e3 Mon Sep 17 00:00:00 2001 From: dependentmadani Date: Sat, 28 Sep 2024 19:30:42 +0100 Subject: [PATCH 21/31] update (test cases): the issue is fixed due to missing an id --- .../postgres/orm/sequelize/noteRelations.ts | 34 +++++++++++-------- 1 file changed, 20 insertions(+), 14 deletions(-) diff --git a/src/repository/storage/postgres/orm/sequelize/noteRelations.ts b/src/repository/storage/postgres/orm/sequelize/noteRelations.ts index e3154121..ee796eef 100644 --- a/src/repository/storage/postgres/orm/sequelize/noteRelations.ts +++ b/src/repository/storage/postgres/orm/sequelize/noteRelations.ts @@ -5,7 +5,7 @@ import type Orm from '@repository/storage/postgres/orm/sequelize/index.js'; import type { NoteInternalId } from '@domain/entities/note.js'; import type { Note } from '@domain/entities/note.js'; import { Model, DataTypes } from 'sequelize'; -import { notEmpty } from '@infrastructure/utils/empty.js'; +import { isEmpty, notEmpty } from '@infrastructure/utils/empty.js'; /** * Class representing a note relations in database @@ -217,21 +217,27 @@ export default class NoteRelationsSequelizeStorage { * @param noteId - the ID of the note. */ public async getAllNoteParentsIds(noteId: NoteInternalId): Promise { - if (!this.noteModel) { - throw new Error('NoteRelationStorage: Note model is not defined'); - } - const parentNotes: NoteInternalId[] = []; + let currentNoteId: number | null = noteId; - // get all note ids via a singe sql query - const noteRelation: NoteRelationsModel[] | null = await this.model.findAll({ - where: { noteId }, - attributes: ['noteId'], - raw: true, - }); - - if (notEmpty(noteRelation)) { - parentNotes.push(...noteRelation.map(relation => relation.noteId)); + // get all note ids via a singe sql query instead of many + while (currentNoteId !== null) { + const noteRelation: NoteRelationsModel | null = await this.model.findOne({ + where: { + noteId: currentNoteId, + }, + }); + + if (notEmpty(noteRelation)) { + parentNotes.push(noteRelation.noteId); + } + + if (isEmpty(noteRelation)) { + parentNotes.push(currentNoteId); + break; + } else { + currentNoteId = noteRelation.parentId ?? null; + } } parentNotes.reverse(); From 5a9fad279383d66b6d8337618307241f7a0578d3 Mon Sep 17 00:00:00 2001 From: dependentmadani Date: Sun, 29 Sep 2024 13:47:22 +0100 Subject: [PATCH 22/31] update (test case): change the naming of a test case --- src/presentation/http/router/note.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/presentation/http/router/note.test.ts b/src/presentation/http/router/note.test.ts index addfe180..88ace91b 100644 --- a/src/presentation/http/router/note.test.ts +++ b/src/presentation/http/router/note.test.ts @@ -640,7 +640,7 @@ describe('Note API', () => { }); }); - test('Returns one parents note in case where the note is not created by the same user but share relation', async () => { + test('Returns one parent\'s note when the user is not in the parent note\'s team but shares a relation', async () => { /** Create test user */ const user = await global.db.insertUser(); From 4ccb075a080225f2a2d971d5ff2fef213d4181d6 Mon Sep 17 00:00:00 2001 From: dependentmadani Date: Sun, 13 Oct 2024 13:14:20 +0100 Subject: [PATCH 23/31] update(note parents): based on last review, still work on sql --- src/presentation/http/router/note.test.ts | 6 +-- src/repository/noteRelations.repository.ts | 2 +- .../postgres/orm/sequelize/noteRelations.ts | 44 +++++++++---------- 3 files changed, 26 insertions(+), 26 deletions(-) diff --git a/src/presentation/http/router/note.test.ts b/src/presentation/http/router/note.test.ts index 88ace91b..b0b49a4a 100644 --- a/src/presentation/http/router/note.test.ts +++ b/src/presentation/http/router/note.test.ts @@ -519,7 +519,7 @@ describe('Note API', () => { expect(response?.json().message).toStrictEqual(expectedMessage); }); - test('Returns one parents note in case when note has one parents', async () => { + test('Returns one parents note in case when note has one parent', async () => { /** Create test user */ const user = await global.db.insertUser(); @@ -572,7 +572,7 @@ describe('Note API', () => { }); }); - test('Returns two parents note in case when note has two parents', async () => { + test('Returns two note parents in case when note has two parents', async () => { /** Create test user */ const user = await global.db.insertUser(); @@ -640,7 +640,7 @@ describe('Note API', () => { }); }); - test('Returns one parent\'s note when the user is not in the parent note\'s team but shares a relation', async () => { + test('Returns one parent note when the user is not in the parent note\'s team but shares a note relation', async () => { /** Create test user */ const user = await global.db.insertUser(); diff --git a/src/repository/noteRelations.repository.ts b/src/repository/noteRelations.repository.ts index 038dc696..cb740640 100644 --- a/src/repository/noteRelations.repository.ts +++ b/src/repository/noteRelations.repository.ts @@ -74,6 +74,6 @@ export default class NoteRelationsRepository { * @returns an array of note parents ids */ public async getNoteParentsIds(noteId: NoteInternalId): Promise { - return await this.storage.getAllNoteParentsIds(noteId); + return await this.storage.getNoteParentsIds(noteId); } } diff --git a/src/repository/storage/postgres/orm/sequelize/noteRelations.ts b/src/repository/storage/postgres/orm/sequelize/noteRelations.ts index ee796eef..7dbfbfc0 100644 --- a/src/repository/storage/postgres/orm/sequelize/noteRelations.ts +++ b/src/repository/storage/postgres/orm/sequelize/noteRelations.ts @@ -1,11 +1,11 @@ import type { CreationOptional, InferAttributes, InferCreationAttributes, ModelStatic, Sequelize } from 'sequelize'; +import { QueryTypes } from 'sequelize'; import { Op } from 'sequelize'; import { NoteModel } from '@repository/storage/postgres/orm/sequelize/note.js'; import type Orm from '@repository/storage/postgres/orm/sequelize/index.js'; import type { NoteInternalId } from '@domain/entities/note.js'; import type { Note } from '@domain/entities/note.js'; import { Model, DataTypes } from 'sequelize'; -import { isEmpty, notEmpty } from '@infrastructure/utils/empty.js'; /** * Class representing a note relations in database @@ -216,29 +216,29 @@ export default class NoteRelationsSequelizeStorage { * where the user has access to. * @param noteId - the ID of the note. */ - public async getAllNoteParentsIds(noteId: NoteInternalId): Promise { - const parentNotes: NoteInternalId[] = []; - let currentNoteId: number | null = noteId; + public async getNoteParentsIds(noteId: NoteInternalId): Promise { + let parentNotes: NoteInternalId[] = []; // get all note ids via a singe sql query instead of many - while (currentNoteId !== null) { - const noteRelation: NoteRelationsModel | null = await this.model.findOne({ - where: { - noteId: currentNoteId, - }, - }); - - if (notEmpty(noteRelation)) { - parentNotes.push(noteRelation.noteId); - } - - if (isEmpty(noteRelation)) { - parentNotes.push(currentNoteId); - break; - } else { - currentNoteId = noteRelation.parentId ?? null; - } - } + const query = ` + WITH RECURSIVE note_tree AS ( + SELECT noteId, parentId + FROM NoteRelations + WHERE noteId = :startNoteId + UNION ALL + SELECT nr.noteId, nr.parentId + FROM NoteRelations nr + INNER JOIN note_tree nt ON nt.parentId = nr.noteId + ) + SELECT noteId FROM note_tree; + `; + + const result = await this.model.sequelize?.query(query, { + replacements: { startNoteId: noteId }, + type: QueryTypes.SELECT, + }); + + parentNotes = (result as { noteId: number }[])?.map(note => note.noteId) ?? []; parentNotes.reverse(); From aa93ee2a6fbc1b4ae3f18f73675492dc8639fc75 Mon Sep 17 00:00:00 2001 From: dependentmadani Date: Tue, 15 Oct 2024 21:23:54 +0100 Subject: [PATCH 24/31] update (parent note): add sql query to retrieve all the parents note without loop --- .../postgres/orm/sequelize/noteRelations.ts | 46 +++++++++++-------- .../storage/postgres/orm/sequelize/teams.ts | 5 -- 2 files changed, 28 insertions(+), 23 deletions(-) diff --git a/src/repository/storage/postgres/orm/sequelize/noteRelations.ts b/src/repository/storage/postgres/orm/sequelize/noteRelations.ts index 7dbfbfc0..54476308 100644 --- a/src/repository/storage/postgres/orm/sequelize/noteRelations.ts +++ b/src/repository/storage/postgres/orm/sequelize/noteRelations.ts @@ -221,26 +221,36 @@ export default class NoteRelationsSequelizeStorage { // get all note ids via a singe sql query instead of many const query = ` - WITH RECURSIVE note_tree AS ( - SELECT noteId, parentId - FROM NoteRelations - WHERE noteId = :startNoteId - UNION ALL - SELECT nr.noteId, nr.parentId - FROM NoteRelations nr - INNER JOIN note_tree nt ON nt.parentId = nr.noteId - ) - SELECT noteId FROM note_tree; - `; - - const result = await this.model.sequelize?.query(query, { - replacements: { startNoteId: noteId }, - type: QueryTypes.SELECT, - }); + WITH RECURSIVE note_parents AS ( + SELECT np.note_id, np.parent_id + FROM ${String(this.database.literal(this.tableName).val)} np + WHERE np.note_id = :startNoteId + UNION ALL + SELECT nr.note_id, nr.parent_id + FROM ${String(this.database.literal(this.tableName).val)} nr + INNER JOIN note_parents np ON np.parent_id = nr.note_id + ) + SELECT np.note_id, np.parent_id + FROM note_parents np;`; + + try { + const result = await this.database.query(query, { + replacements: { startNoteId: noteId }, + type: QueryTypes.SELECT, + }); - parentNotes = (result as { noteId: number }[])?.map(note => note.noteId) ?? []; + // eslint-disable-next-line @typescript-eslint/naming-convention + const output = (result as { note_id: number; parent_id: number }[])?.map(note => note.parent_id) ?? []; - parentNotes.reverse(); + if (output.find(note => (note == noteId)) == undefined) { + parentNotes = [noteId, ...output]; + } else { + parentNotes = output; + } + parentNotes.reverse(); + } catch { + console.log(`something wrong happened with sql query`); + } return parentNotes; } diff --git a/src/repository/storage/postgres/orm/sequelize/teams.ts b/src/repository/storage/postgres/orm/sequelize/teams.ts index 824280f0..5dfc0cd5 100644 --- a/src/repository/storage/postgres/orm/sequelize/teams.ts +++ b/src/repository/storage/postgres/orm/sequelize/teams.ts @@ -31,11 +31,6 @@ export class TeamsModel extends Model, InferCreation * Team member role, show what user can do with note */ public declare role: MemberRole; - - /** - * Note relation content - */ - public declare notes?: NoteModel | null; } /** From ed49e60819c37a95f1439d64dbd917209cdad7cd Mon Sep 17 00:00:00 2001 From: dependentmadani Date: Tue, 15 Oct 2024 21:38:12 +0100 Subject: [PATCH 25/31] update (note parents): only include the parents note without the main note --- src/presentation/http/router/note.test.ts | 21 ++----------------- .../postgres/orm/sequelize/noteRelations.ts | 7 +------ 2 files changed, 3 insertions(+), 25 deletions(-) diff --git a/src/presentation/http/router/note.test.ts b/src/presentation/http/router/note.test.ts index b0b49a4a..732accb1 100644 --- a/src/presentation/http/router/note.test.ts +++ b/src/presentation/http/router/note.test.ts @@ -564,10 +564,6 @@ describe('Note API', () => { id: parentNote.publicId, content: parentNote.content, }, - { - id: childNote.publicId, - content: childNote.content, - }, ], }); }); @@ -632,10 +628,6 @@ describe('Note API', () => { id: parentNote.publicId, content: parentNote.content, }, - { - id: childNote.publicId, - content: childNote.content, - }, ], }); }); @@ -688,15 +680,11 @@ describe('Note API', () => { id: parentNote.publicId, content: parentNote.content, }, - { - id: childNote.publicId, - content: childNote.content, - }, ], }); }); - test('Returns one note in case where there is no relation exist for the note with status 200', async () => { + test('Returns no note in case where there is no relation exist for the note with status 200', async () => { /** Create test user */ const user = await global.db.insertUser(); @@ -725,12 +713,7 @@ describe('Note API', () => { expect(response?.statusCode).toBe(200); expect(response?.json()).toMatchObject({ - parents: [ - { - id: note.publicId, - content: note.content, - }, - ], + parents: [], }); }); }); diff --git a/src/repository/storage/postgres/orm/sequelize/noteRelations.ts b/src/repository/storage/postgres/orm/sequelize/noteRelations.ts index 54476308..8031c9d1 100644 --- a/src/repository/storage/postgres/orm/sequelize/noteRelations.ts +++ b/src/repository/storage/postgres/orm/sequelize/noteRelations.ts @@ -240,13 +240,8 @@ export default class NoteRelationsSequelizeStorage { }); // eslint-disable-next-line @typescript-eslint/naming-convention - const output = (result as { note_id: number; parent_id: number }[])?.map(note => note.parent_id) ?? []; + parentNotes = (result as { note_id: number; parent_id: number }[])?.map(note => note.parent_id) ?? []; - if (output.find(note => (note == noteId)) == undefined) { - parentNotes = [noteId, ...output]; - } else { - parentNotes = output; - } parentNotes.reverse(); } catch { console.log(`something wrong happened with sql query`); From 7bfe0eb440668a747f3fd9db4116e11170709ef9 Mon Sep 17 00:00:00 2001 From: dependentmadani Date: Wed, 16 Oct 2024 14:44:43 +0100 Subject: [PATCH 26/31] update (note parents): modify based on last review --- src/domain/service/note.ts | 8 ++----- src/presentation/http/router/note.test.ts | 12 +++++----- src/presentation/http/router/note.ts | 6 ++++- .../storage/postgres/orm/sequelize/note.ts | 21 +++++++----------- .../postgres/orm/sequelize/noteRelations.ts | 22 +++++++------------ 5 files changed, 29 insertions(+), 40 deletions(-) diff --git a/src/domain/service/note.ts b/src/domain/service/note.ts index 42ae2b7b..6ae5a41e 100644 --- a/src/domain/service/note.ts +++ b/src/domain/service/note.ts @@ -9,7 +9,6 @@ import type User from '@domain/entities/user.js'; import type { NoteList } from '@domain/entities/noteList.js'; import type NoteHistoryRepository from '@repository/noteHistory.repository.js'; import type { NoteHistoryMeta, NoteHistoryRecord, NoteHistoryPublic } from '@domain/entities/noteHistory.js'; -import { definePublicNote, type NotePublic } from '@domain/entities/notePublic.js'; /** * Note service @@ -449,13 +448,10 @@ export default class NoteService { * @param noteId - id of the note to get parent structure * @returns - array of notes that are parent structure of the note */ - public async getNoteParents(noteId: NoteInternalId): Promise { + public async getNoteParents(noteId: NoteInternalId): Promise { const noteIds: NoteInternalId[] = await this.noteRelationsRepository.getNoteParentsIds(noteId); const noteParents = await this.noteRepository.getNotesByIds(noteIds); - const noteParentsPublic: NotePublic[] = noteParents.map((note) => { - return definePublicNote(note); - }); - return noteParentsPublic; + return noteParents; } } diff --git a/src/presentation/http/router/note.test.ts b/src/presentation/http/router/note.test.ts index 732accb1..240e27d2 100644 --- a/src/presentation/http/router/note.test.ts +++ b/src/presentation/http/router/note.test.ts @@ -523,7 +523,7 @@ describe('Note API', () => { /** Create test user */ const user = await global.db.insertUser(); - /** Create acces token for the user */ + /** Create access token for the user */ const accessToken = global.auth(user.id); /** Create test note - a parent note */ @@ -572,7 +572,7 @@ describe('Note API', () => { /** Create test user */ const user = await global.db.insertUser(); - /** Create acces token for the user */ + /** Create access token for the user */ const accessToken = global.auth(user.id); /** Create test note - a grand parent note */ @@ -632,14 +632,14 @@ describe('Note API', () => { }); }); - test('Returns one parent note when the user is not in the parent note\'s team but shares a note relation', async () => { + test('Returns one parent note when the user is not in the parent note\'s team but has note relation', async () => { /** Create test user */ const user = await global.db.insertUser(); /** Create another user */ const anotherUser = await global.db.insertUser(); - /** Create acces token for the user */ + /** Create access token for the user */ const accessToken = global.auth(user.id); /** Create test note - a parent note */ @@ -684,11 +684,11 @@ describe('Note API', () => { }); }); - test('Returns no note in case where there is no relation exist for the note with status 200', async () => { + test('Returns empty array in case where there is no relation exist for the note with status 200', async () => { /** Create test user */ const user = await global.db.insertUser(); - /** Create acces token for the user */ + /** Create access token for the user */ const accessToken = global.auth(user.id); /** Create test note - a child note */ diff --git a/src/presentation/http/router/note.ts b/src/presentation/http/router/note.ts index 1d2a715d..3427890b 100644 --- a/src/presentation/http/router/note.ts +++ b/src/presentation/http/router/note.ts @@ -181,12 +181,16 @@ const NoteRouter: FastifyPluginCallback = (fastify, opts, don const noteParentStructure = await noteService.getNoteParents(noteId); + const noteParentsPublic = noteParentStructure.map((notes) => { + return definePublicNote(notes); + }); + return reply.send({ note: notePublic, parentNote: parentNote, accessRights: { canEdit: canEdit }, tools: noteTools, - parents: noteParentStructure, + parents: noteParentsPublic, }); }); diff --git a/src/repository/storage/postgres/orm/sequelize/note.ts b/src/repository/storage/postgres/orm/sequelize/note.ts index d789130e..15b9f8e2 100644 --- a/src/repository/storage/postgres/orm/sequelize/note.ts +++ b/src/repository/storage/postgres/orm/sequelize/note.ts @@ -1,12 +1,11 @@ import type { CreationOptional, InferAttributes, InferCreationAttributes, ModelStatic, NonAttribute, Sequelize } from 'sequelize'; -import { DataTypes, Model } from 'sequelize'; +import { DataTypes, Model, Op } from 'sequelize'; import type Orm from '@repository/storage/postgres/orm/sequelize/index.js'; import type { Note, NoteCreationAttributes, NoteInternalId, NotePublicId } from '@domain/entities/note.js'; import { UserModel } from '@repository/storage/postgres/orm/sequelize/user.js'; import type { NoteSettingsModel } from './noteSettings.js'; import type { NoteVisitsModel } from './noteVisits.js'; import type { NoteHistoryModel } from './noteHistory.js'; -import { notEmpty } from '@infrastructure/utils/empty.js'; /* eslint-disable @typescript-eslint/naming-convention */ @@ -330,17 +329,13 @@ export default class NoteSequelizeStorage { * @param noteIds - list of note ids */ public async getNotesByIds(noteIds: NoteInternalId[]): Promise { - const notes: Note[] = []; - - for (const noteId of noteIds) { - const note: Note | null = await this.model.findOne({ - where: { id: noteId }, - }); - - if (notEmpty(note)) { - notes.push(note); - } - } + const notes: Note[] = await this.model.findAll({ + where: { + id: { + [Op.in]: noteIds, + }, + }, + }); return notes; } diff --git a/src/repository/storage/postgres/orm/sequelize/noteRelations.ts b/src/repository/storage/postgres/orm/sequelize/noteRelations.ts index 8031c9d1..c6ed962d 100644 --- a/src/repository/storage/postgres/orm/sequelize/noteRelations.ts +++ b/src/repository/storage/postgres/orm/sequelize/noteRelations.ts @@ -217,8 +217,6 @@ export default class NoteRelationsSequelizeStorage { * @param noteId - the ID of the note. */ public async getNoteParentsIds(noteId: NoteInternalId): Promise { - let parentNotes: NoteInternalId[] = []; - // get all note ids via a singe sql query instead of many const query = ` WITH RECURSIVE note_parents AS ( @@ -233,20 +231,16 @@ export default class NoteRelationsSequelizeStorage { SELECT np.note_id, np.parent_id FROM note_parents np;`; - try { - const result = await this.database.query(query, { - replacements: { startNoteId: noteId }, - type: QueryTypes.SELECT, - }); + const result = await this.database.query(query, { + replacements: { startNoteId: noteId }, + type: QueryTypes.SELECT, + }); - // eslint-disable-next-line @typescript-eslint/naming-convention - parentNotes = (result as { note_id: number; parent_id: number }[])?.map(note => note.parent_id) ?? []; + // eslint-disable-next-line @typescript-eslint/naming-convention + let noteParents = (result as { note_id: number; parent_id: number }[])?.map(note => note.parent_id) ?? []; - parentNotes.reverse(); - } catch { - console.log(`something wrong happened with sql query`); - } + noteParents.reverse(); - return parentNotes; + return noteParents; } } From dd1bcdc467515b1f7ccd59a38c1fb06fd782c31c Mon Sep 17 00:00:00 2001 From: dependentmadani Date: Wed, 16 Oct 2024 21:10:02 +0100 Subject: [PATCH 27/31] update (note relation): return noteId and parentId from the sql query --- .../storage/postgres/orm/sequelize/noteRelations.ts | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/repository/storage/postgres/orm/sequelize/noteRelations.ts b/src/repository/storage/postgres/orm/sequelize/noteRelations.ts index c6ed962d..f7f24b34 100644 --- a/src/repository/storage/postgres/orm/sequelize/noteRelations.ts +++ b/src/repository/storage/postgres/orm/sequelize/noteRelations.ts @@ -228,16 +228,15 @@ export default class NoteRelationsSequelizeStorage { FROM ${String(this.database.literal(this.tableName).val)} nr INNER JOIN note_parents np ON np.parent_id = nr.note_id ) - SELECT np.note_id, np.parent_id + SELECT np.note_id AS "noteId", np.parent_id AS "parentId" FROM note_parents np;`; - const result = await this.database.query(query, { + const result = await this.model.sequelize?.query(query, { replacements: { startNoteId: noteId }, type: QueryTypes.SELECT, }); - // eslint-disable-next-line @typescript-eslint/naming-convention - let noteParents = (result as { note_id: number; parent_id: number }[])?.map(note => note.parent_id) ?? []; + let noteParents = (result as { noteId: number; parentId: number }[])?.map(note => note.parentId) ?? []; noteParents.reverse(); From 92c856953f4e8ddc0eba42a7cfb2fc71a1457476 Mon Sep 17 00:00:00 2001 From: dependentmadani Date: Thu, 17 Oct 2024 14:22:49 +0100 Subject: [PATCH 28/31] update (note parents): update the search of note ids to respect the order, add related test to this case --- src/domain/service/note.ts | 3 +- src/presentation/http/router/note.test.ts | 30 +++++++++---------- .../storage/postgres/orm/sequelize/note.ts | 7 +++++ 3 files changed, 23 insertions(+), 17 deletions(-) diff --git a/src/domain/service/note.ts b/src/domain/service/note.ts index 6ae5a41e..95976baf 100644 --- a/src/domain/service/note.ts +++ b/src/domain/service/note.ts @@ -443,8 +443,7 @@ export default class NoteService { } /** - * Get note parent structure recursively by note id and user id - * and check if user has access to the parent note. + * Return a sequence of parent notes for the given note id. * @param noteId - id of the note to get parent structure * @returns - array of notes that are parent structure of the note */ diff --git a/src/presentation/http/router/note.test.ts b/src/presentation/http/router/note.test.ts index 240e27d2..8488d95d 100644 --- a/src/presentation/http/router/note.test.ts +++ b/src/presentation/http/router/note.test.ts @@ -568,7 +568,7 @@ describe('Note API', () => { }); }); - test('Returns two note parents in case when note has two parents', async () => { + test('Returns two note parents in case when the note parents IDs relations are provided in a different order than expected', async () => { /** Create test user */ const user = await global.db.insertUser(); @@ -576,36 +576,36 @@ describe('Note API', () => { const accessToken = global.auth(user.id); /** Create test note - a grand parent note */ - const grandParentNote = await global.db.insertNote({ + const firstNote = await global.db.insertNote({ creatorId: user.id, }); /** Create test note - a parent note */ - const parentNote = await global.db.insertNote({ + const secondNote = await global.db.insertNote({ creatorId: user.id, }); /** Create test note - a child note */ - const childNote = await global.db.insertNote({ + const thirdNote = await global.db.insertNote({ creatorId: user.id, }); /** Create test note settings */ await global.db.insertNoteSetting({ - noteId: childNote.id, + noteId: secondNote.id, isPublic: true, }); /** Create note relation between parent and grandParentNote */ await global.db.insertNoteRelation({ - parentId: grandParentNote.id, - noteId: parentNote.id, + parentId: firstNote.id, + noteId: thirdNote.id, }); /** Create test note relation */ await global.db.insertNoteRelation({ - parentId: parentNote.id, - noteId: childNote.id, + parentId: thirdNote.id, + noteId: secondNote.id, }); const response = await global.api?.fakeRequest({ @@ -613,7 +613,7 @@ describe('Note API', () => { headers: { authorization: `Bearer ${accessToken}`, }, - url: `/note/${childNote.publicId}`, + url: `/note/${secondNote.publicId}`, }); expect(response?.statusCode).toBe(200); @@ -621,12 +621,12 @@ describe('Note API', () => { expect(response?.json()).toMatchObject({ parents: [ { - id: grandParentNote.publicId, - content: grandParentNote.content, + id: firstNote.publicId, + content: firstNote.content, }, { - id: parentNote.publicId, - content: parentNote.content, + id: thirdNote.publicId, + content: thirdNote.content, }, ], }); @@ -684,7 +684,7 @@ describe('Note API', () => { }); }); - test('Returns empty array in case where there is no relation exist for the note with status 200', async () => { + test('Returns empty array in case where there is no relation exist for the note', async () => { /** Create test user */ const user = await global.db.insertUser(); diff --git a/src/repository/storage/postgres/orm/sequelize/note.ts b/src/repository/storage/postgres/orm/sequelize/note.ts index 15b9f8e2..f2c329d9 100644 --- a/src/repository/storage/postgres/orm/sequelize/note.ts +++ b/src/repository/storage/postgres/orm/sequelize/note.ts @@ -329,12 +329,19 @@ export default class NoteSequelizeStorage { * @param noteIds - list of note ids */ public async getNotesByIds(noteIds: NoteInternalId[]): Promise { + if (noteIds.length === 0) { + return []; + } + const notes: Note[] = await this.model.findAll({ where: { id: { [Op.in]: noteIds, }, }, + order: [ + this.database.literal(`ARRAY_POSITION(ARRAY[${noteIds.map(id => `${id}`).join(',')}], id)`), + ], }); return notes; From 7efadf5e0bfc307e83ffe23836ae3eb88cd3d92f Mon Sep 17 00:00:00 2001 From: dependentmadani Date: Tue, 22 Oct 2024 13:46:37 +0100 Subject: [PATCH 29/31] update (note parents): add comment about the query --- .../storage/postgres/orm/sequelize/noteRelations.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/repository/storage/postgres/orm/sequelize/noteRelations.ts b/src/repository/storage/postgres/orm/sequelize/noteRelations.ts index f7f24b34..e7eb9aa8 100644 --- a/src/repository/storage/postgres/orm/sequelize/noteRelations.ts +++ b/src/repository/storage/postgres/orm/sequelize/noteRelations.ts @@ -217,7 +217,10 @@ export default class NoteRelationsSequelizeStorage { * @param noteId - the ID of the note. */ public async getNoteParentsIds(noteId: NoteInternalId): Promise { - // get all note ids via a singe sql query instead of many + // Query to get all parent notes of a note. + // The query uses a recursive common table expression (CTE) to get all parent notes of a note. + // It starts from the note with the ID :startNoteId and recursively gets all parent notes. + // It returns a list of note ID and parent ID of the note. const query = ` WITH RECURSIVE note_parents AS ( SELECT np.note_id, np.parent_id From 47680d64a6230b89167ae7770df9b1a967b15b73 Mon Sep 17 00:00:00 2001 From: dependentmadani Date: Thu, 24 Oct 2024 20:38:51 +0100 Subject: [PATCH 30/31] update (note parents): small modification of function description and test naming --- src/presentation/http/router/note.test.ts | 2 +- src/repository/storage/postgres/orm/sequelize/note.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/presentation/http/router/note.test.ts b/src/presentation/http/router/note.test.ts index 8488d95d..139d7d34 100644 --- a/src/presentation/http/router/note.test.ts +++ b/src/presentation/http/router/note.test.ts @@ -568,7 +568,7 @@ describe('Note API', () => { }); }); - test('Returns two note parents in case when the note parents IDs relations are provided in a different order than expected', async () => { + test('Returns note parents in correct order in case when parents created in a non-linear order', async () => { /** Create test user */ const user = await global.db.insertUser(); diff --git a/src/repository/storage/postgres/orm/sequelize/note.ts b/src/repository/storage/postgres/orm/sequelize/note.ts index f2c329d9..c45b3be5 100644 --- a/src/repository/storage/postgres/orm/sequelize/note.ts +++ b/src/repository/storage/postgres/orm/sequelize/note.ts @@ -325,7 +325,7 @@ export default class NoteSequelizeStorage { }; /** - * Get all notes based on their ids + * Get all notes based on their ids in the same order of passed ids * @param noteIds - list of note ids */ public async getNotesByIds(noteIds: NoteInternalId[]): Promise { From 31aeb92c80632a9809e12baad312363be6bc680f Mon Sep 17 00:00:00 2001 From: dependentmadani Date: Thu, 24 Oct 2024 20:50:01 +0100 Subject: [PATCH 31/31] remove (note parents test): an unecessary test case has been removed --- src/presentation/http/router/note.test.ts | 52 ----------------------- 1 file changed, 52 deletions(-) diff --git a/src/presentation/http/router/note.test.ts b/src/presentation/http/router/note.test.ts index 139d7d34..9f0a9a5b 100644 --- a/src/presentation/http/router/note.test.ts +++ b/src/presentation/http/router/note.test.ts @@ -632,58 +632,6 @@ describe('Note API', () => { }); }); - test('Returns one parent note when the user is not in the parent note\'s team but has note relation', async () => { - /** Create test user */ - const user = await global.db.insertUser(); - - /** Create another user */ - const anotherUser = await global.db.insertUser(); - - /** Create access token for the user */ - const accessToken = global.auth(user.id); - - /** Create test note - a parent note */ - const parentNote = await global.db.insertNote({ - creatorId: anotherUser.id, - }); - - /** Create test note - a child note */ - const childNote = await global.db.insertNote({ - creatorId: user.id, - }); - - /** Create test note settings */ - await global.db.insertNoteSetting({ - noteId: childNote.id, - isPublic: true, - }); - - /** Create test note relation */ - await global.db.insertNoteRelation({ - parentId: parentNote.id, - noteId: childNote.id, - }); - - const response = await global.api?.fakeRequest({ - method: 'GET', - headers: { - authorization: `Bearer ${accessToken}`, - }, - url: `/note/${childNote.publicId}`, - }); - - expect(response?.statusCode).toBe(200); - - expect(response?.json()).toMatchObject({ - parents: [ - { - id: parentNote.publicId, - content: parentNote.content, - }, - ], - }); - }); - test('Returns empty array in case where there is no relation exist for the note', async () => { /** Create test user */ const user = await global.db.insertUser();