Skip to content

Commit

Permalink
udpate (note parents): based on last review
Browse files Browse the repository at this point in the history
  • Loading branch information
dependentmadani committed Sep 27, 2024
1 parent fee9bbf commit 14234a1
Show file tree
Hide file tree
Showing 7 changed files with 55 additions and 127 deletions.
2 changes: 1 addition & 1 deletion src/domain/service/note.ts
Original file line number Diff line number Diff line change
Expand Up @@ -451,7 +451,7 @@ export default class NoteService {
*/
public async getNoteParents(noteId: NoteInternalId): Promise<NotePublic[]> {
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);
});
Expand Down
76 changes: 15 additions & 61 deletions src/presentation/http/router/note.test.ts
Original file line number Diff line number Diff line change
@@ -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', () => {
/**
Expand Down Expand Up @@ -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 */
{
Expand Down Expand Up @@ -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();

Expand Down Expand Up @@ -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();

Expand Down Expand Up @@ -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();

Expand Down Expand Up @@ -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,
},
],
Expand Down Expand Up @@ -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,
},
],
Expand Down
1 change: 0 additions & 1 deletion src/repository/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,6 @@ export async function init(orm: Orm, s3Config: S3StorageConfig): Promise<Reposit
/**
* Create associations between note and relations table
*/
noteStorage.createAssociationWithNoteRelationModel(noteRelationshipStorage.model);
noteRelationshipStorage.createAssociationWithNoteModel(noteStorage.model);

/**
Expand Down
9 changes: 9 additions & 0 deletions src/repository/note.repository.ts
Original file line number Diff line number Diff line change
Expand Up @@ -81,4 +81,13 @@ export default class NoteRepository {
public async getNoteListByUserId(id: number, offset: number, limit: number): Promise<Note[]> {
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<Note[]> {
return await this.storage.getNotesByIds(noteIds);
}
}
11 changes: 1 addition & 10 deletions src/repository/noteRelations.repository.ts
Original file line number Diff line number Diff line change
@@ -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';

/**
Expand Down Expand Up @@ -76,13 +76,4 @@ export default class NoteRelationsRepository {
public async getNoteParentsIds(noteId: NoteInternalId): Promise<NoteInternalId[]> {
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<Note[]> {
return await this.storage.getNotesByIds(noteIds);
}
}
31 changes: 21 additions & 10 deletions src/repository/storage/postgres/orm/sequelize/note.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 */

Expand Down Expand Up @@ -76,8 +76,6 @@ export default class NoteSequelizeStorage {

public historyModel: typeof NoteHistoryModel | null = null;

public noteRelationModel: typeof NoteRelationsModel | null = null;

/**
* Database instance
*/
Expand Down Expand Up @@ -158,13 +156,6 @@ export default class NoteSequelizeStorage {
});
};

public createAssociationWithNoteRelationModel(model: ModelStatic<NoteRelationsModel>): void {
/**
* Create association with note relations
*/
this.noteRelationModel = model;
}

/**
* Insert note to database
* @param options - note creation options
Expand Down Expand Up @@ -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<Note[]> {
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;
}
}
52 changes: 8 additions & 44 deletions src/repository/storage/postgres/orm/sequelize/noteRelations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<Note[]> {
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;
}
}

0 comments on commit 14234a1

Please sign in to comment.