From c27c44dc7b5cee8d96c384969dd2417bb01f55e8 Mon Sep 17 00:00:00 2001 From: Marina Kim Date: Sun, 19 Oct 2025 15:05:16 +0100 Subject: [PATCH 1/3] update PUT and POST mentor with tags --- repositories/mentor_repository.js | 73 ++++++++++++++++++++++----- repositories/tags_repository.js | 48 ++++++++++++++++++ routes/mentors.js | 1 + services/mentor_service.js | 2 + tests/services/mentor_service.test.js | 4 +- utils.js | 2 +- 6 files changed, 115 insertions(+), 15 deletions(-) diff --git a/repositories/mentor_repository.js b/repositories/mentor_repository.js index bd07ca2..4b918df 100644 --- a/repositories/mentor_repository.js +++ b/repositories/mentor_repository.js @@ -1,4 +1,5 @@ const { getKnex } = require("../db"); +const tagsRepo = require("./tags_repository"); async function getMentors(statusesFilter, selectedFields) { const knex = getKnex(); @@ -19,34 +20,80 @@ async function getMentorByUserId(userId) { async function getMentorById(allowedStatuses, selectedFields, mentorId) { const knex = getKnex(); - return await knex("mentors") + const mentor = await knex("mentors") .join("users", "mentors.user_id", "users.id") .where("mentors.id", mentorId) .whereIn("mentors.status", allowedStatuses) .select(selectedFields).first(); + + const mentorTags = await tagsRepo.getAssignedTagNames( + mentorId, + 'mentor' + ); + + mentor.tags = mentorTags; + + return mentor; } async function createMentor(mentorData) { const knex = getKnex(); + const { tags = [], ...data } = mentorData; - const [createdMentor] = await knex("mentors") - .insert(mentorData) - .returning("id"); + return await knex.transaction(async (trx) => { + const [createdMentor] = await trx("mentors") + .insert(data) + .returning("id"); - const result = await knex("mentors") - .join("users", "mentors.user_id", "users.id") - .select("mentors.id", "users.name", "mentors.about") - .where("mentors.id", createdMentor.id) - .first(); + await updateMentorTags(createdMentor.id, tags, trx); + + const result = await trx("mentors") + .join("users", "mentors.user_id", "users.id") + .select("mentors.id", "users.name", "mentors.about") + .where("mentors.id", createdMentor.id) + .first(); - return result; + return result; + }); +} + +async function updateMentorTags(mentorId, tags = [], trx) { + const existing = await tagsRepo.getAssignedTagNames(mentorId, 'mentor', trx); + + const toRemove = existing.filter((name) => !tags.includes(name)); + const toAdd = tags.filter((name) => !existing.includes(name)); + + if (toRemove.length) { + const idsToRemove = await Promise.all( + toRemove.map((name) => + tagsRepo.findTagByName(name, trx).then((t) => t.id) + ) + ); + await tagsRepo.removeAssignments(idsToRemove, mentorId, 'mentor', trx); + } + + for (const name of toAdd) { + const tag = await tagsRepo.ensureTag(name, trx); + await tagsRepo.assignTagTo(tag.id, mentorId, 'mentor', trx); + } } async function updateMentorById(mentorId, updates) { const knex = getKnex(); - return await knex("mentors") - .where("id", mentorId) - .update(updates); + const { tags = [], ...mentorUpdates } = updates; + + return await knex.transaction(async (trx) => { + // if other updates besides tags, then update mentor + if (Object.keys(mentorUpdates).length) { + await trx("mentors") + .where("id", mentorId) + .update(mentorUpdates); + } + + await updateMentorTags(mentorId, tags, trx); + + return mentorId; + }); } async function deleteMentorById(id) { diff --git a/repositories/tags_repository.js b/repositories/tags_repository.js index c8257b6..0210683 100644 --- a/repositories/tags_repository.js +++ b/repositories/tags_repository.js @@ -6,6 +6,54 @@ async function getAllTags() { .select('id', 'name'); } +async function findTagByName(name, trx) { + return await (trx || getKnex())('tags') + .where('name', name) + .first(); +} + +async function createTag(name, trx) { + const [{ id }] = await (trx || getKnex())('tags') + .insert({ name }) + .returning('id'); + return { id, name }; +} + +async function ensureTag(name, trx) { + let tag = await findTagByName(name, trx); + if (!tag) { + tag = await createTag(name, trx); + } + return tag; +} + +async function assignTagTo(tagId, assignedId, assignedTo, trx) { + return await (trx || getKnex())('tags_assigned') + .insert({ tag_id: tagId, assigned_id: assignedId, assigned_to: assignedTo }); +} + +async function removeAssignments(tagIds, assignedId, assignedTo, trx) { + if (!tagIds.length) return; + return await (trx || getKnex())('tags_assigned') + .where('assigned_id', assignedId) + .andWhere('assigned_to', assignedTo) + .whereIn('tag_id', tagIds) + .del(); +} + +async function getAssignedTagNames(assignedId, assignedTo, trx) { + return await (trx || getKnex())('tags_assigned as ta') + .join('tags as t', 'ta.tag_id', 't.id') + .where({ 'ta.assigned_id': assignedId, 'ta.assigned_to': assignedTo }) + .pluck('t.name'); +} + module.exports = { getAllTags, + findTagByName, + createTag, + ensureTag, + assignTagTo, + removeAssignments, + getAssignedTagNames, }; \ No newline at end of file diff --git a/routes/mentors.js b/routes/mentors.js index d8aa5aa..f05ed38 100644 --- a/routes/mentors.js +++ b/routes/mentors.js @@ -18,6 +18,7 @@ router.post("/mentors", verifyRoles([ROLE_NAMES.member]), async (req, res) => { const mentorData = { about: req.body.about, contact: req.body.contact, + tags: req.body.tags }; const mentorId = await mentorService.createMentor(req.userId, mentorData); diff --git a/services/mentor_service.js b/services/mentor_service.js index d1d5a57..7e4fb05 100644 --- a/services/mentor_service.js +++ b/services/mentor_service.js @@ -10,6 +10,7 @@ const baseFields = [ 'mentors.user_id', 'mentors.about', 'mentors.status', + 'mentors.tags', 'users.name', 'users.img as img_link' ]; @@ -36,6 +37,7 @@ async function createMentor(userId, mentorData) { status: 'pending', about: mentorData.about, contact: mentorData.contact, + tags: mentorData.tags }; const mentorInfo = await repo.createMentor(mentor); diff --git a/tests/services/mentor_service.test.js b/tests/services/mentor_service.test.js index 4dea1c5..6e5908b 100644 --- a/tests/services/mentor_service.test.js +++ b/tests/services/mentor_service.test.js @@ -13,6 +13,7 @@ jest.mock('../../services/mailer_service'); const defaultUserId = 42; const mockedMentorId = 1; +const mockedTags = ["React", "PHP"]; describe('Mentor Service', () => { afterEach(() => { @@ -21,7 +22,7 @@ describe('Mentor Service', () => { describe('createMentor', () => { it('Create mentor in pending state success', async () => { - const mentorData = { about: 'I am a mentor', contact: 'mentor@example.com' }; + const mentorData = { about: 'I am a mentor', contact: 'mentor@example.com', tags: mockedTags }; const createdMentor = { id: 1, name: 'Name', about: mentorData.about }; repo.getMentorByUserId.mockResolvedValue(null); repo.createMentor.mockResolvedValue(createdMentor); @@ -34,6 +35,7 @@ describe('Mentor Service', () => { status: MENTOR_STATUSES.pending, about: mentorData.about, contact: mentorData.contact, + tags: mentorData.tags }); expect(rolesService.getAdminEmails).toHaveBeenCalled(); expect(mailerService.sendEmailOnNewMentorApplication).toHaveBeenCalled(); diff --git a/utils.js b/utils.js index 3df31d4..5e0e58b 100644 --- a/utils.js +++ b/utils.js @@ -74,7 +74,7 @@ const MENTOR_STATUSES = { pending: 'pending', }; -const ALLOWED_MENTOR_UPDATES = ['contact', 'about']; +const ALLOWED_MENTOR_UPDATES = ['contact', 'about', 'tags']; module.exports = { APPL_STATUSES, MENTOR_STATUSES, ROLE_NAMES, ALLOWED_MENTOR_UPDATES, From 4a0f8e3e6a445c1fd45a6037b5e80d3d53cb51fb Mon Sep 17 00:00:00 2001 From: Marina Kim Date: Mon, 27 Oct 2025 12:12:16 +0000 Subject: [PATCH 2/3] updates tests for tags --- repositories/mentor_repository.js | 53 +++------------------ repositories/tags_repository.js | 67 ++++++++++++++++++--------- routes/mentors.js | 3 ++ services/mentor_service.js | 25 ++++++++-- tests/services/mentor_service.test.js | 28 ++++++++++- 5 files changed, 101 insertions(+), 75 deletions(-) diff --git a/repositories/mentor_repository.js b/repositories/mentor_repository.js index f1919a0..905dc46 100644 --- a/repositories/mentor_repository.js +++ b/repositories/mentor_repository.js @@ -1,5 +1,4 @@ const { getKnex } = require("../db"); -const tagsRepo = require("./tags_repository"); async function getMentors(statusesFilter, selectedFields) { const knex = getKnex(); @@ -25,28 +24,18 @@ async function getMentorById(allowedStatuses, selectedFields, mentorId) { .where("mentors.id", mentorId) .whereIn("mentors.status", allowedStatuses) .select(selectedFields).first(); - - const mentorTags = await tagsRepo.getAssignedTagNames( - mentorId, - 'mentor' - ); - - mentor.tags = mentorTags; return mentor; } async function createMentor(mentorData) { const knex = getKnex(); - const { tags = [], ...data } = mentorData; return await knex.transaction(async (trx) => { const [createdMentor] = await trx("mentors") - .insert(data) + .insert(mentorData) .returning("id"); - await updateMentorTags(createdMentor.id, tags, trx); - const result = await trx("mentors") .join("users", "mentors.user_id", "users.id") .select("mentors.id", "users.name", "mentors.about") @@ -57,44 +46,14 @@ async function createMentor(mentorData) { }); } -async function updateMentorTags(mentorId, tags = [], trx) { - const existing = await tagsRepo.getAssignedTagNames(mentorId, 'mentor', trx); - - const toRemove = existing.filter((name) => !tags.includes(name)); - const toAdd = tags.filter((name) => !existing.includes(name)); - - if (toRemove.length) { - const idsToRemove = await Promise.all( - toRemove.map((name) => - tagsRepo.findTagByName(name, trx).then((t) => t.id) - ) - ); - await tagsRepo.removeAssignments(idsToRemove, mentorId, 'mentor', trx); - } - - for (const name of toAdd) { - const tag = await tagsRepo.ensureTag(name, trx); - await tagsRepo.assignTagTo(tag.id, mentorId, 'mentor', trx); - } -} - async function updateMentorById(mentorId, updates) { const knex = getKnex(); - const { tags = [], ...mentorUpdates } = updates; - - return await knex.transaction(async (trx) => { - await updateMentorTags(mentorId, tags, trx); - // if other updates besides tags, then update mentor - if (Object.keys(mentorUpdates).length) { - const [mentor] = await trx("mentors") - .where("id", mentorId) - .update(mentorUpdates) - .returning("id"); - return mentor.id; - } - return mentorId; - }) + const [mentor] = await knex("mentors") + .where("id", mentorId) + .update(updates) + .returning("id"); + return mentor.id; } async function deleteMentorById(id) { diff --git a/repositories/tags_repository.js b/repositories/tags_repository.js index 199c450..5e6300f 100644 --- a/repositories/tags_repository.js +++ b/repositories/tags_repository.js @@ -6,54 +6,77 @@ async function getAllTags() { return names; } -async function findTagByName(name, trx) { - return await (trx || getKnex())('tags') +async function getTagByName(name) { + const knex = getKnex(); + return await knex('tags') .where('name', name) .first(); } -async function createTag(name, trx) { - const [{ id }] = await (trx || getKnex())('tags') +async function createTag(name) { + const knex = getKnex(); + const [{ id }] = await knex('tags') .insert({ name }) .returning('id'); return { id, name }; } -async function ensureTag(name, trx) { - let tag = await findTagByName(name, trx); +async function ensureTag(name) { + let tag = await getTagByName(name); if (!tag) { - tag = await createTag(name, trx); + tag = await createTag(name); } return tag; } -async function assignTagTo(tagId, assignedId, assignedTo, trx) { - return await (trx || getKnex())('tags_assigned') +async function assignTagTo(tagId, assignedId, assignedTo) { + const knex = getKnex(); + return await knex('tags_assigned') .insert({ tag_id: tagId, assigned_id: assignedId, assigned_to: assignedTo }); } -async function removeAssignments(tagIds, assignedId, assignedTo, trx) { - if (!tagIds.length) return; - return await (trx || getKnex())('tags_assigned') - .where('assigned_id', assignedId) - .andWhere('assigned_to', assignedTo) - .whereIn('tag_id', tagIds) - .del(); -} - -async function getAssignedTagNames(assignedId, assignedTo, trx) { - return await (trx || getKnex())('tags_assigned as ta') +async function getAssignedTagNames(assignedId, assignedTo) { + const knex = getKnex(); + return await knex('tags_assigned as ta') .join('tags as t', 'ta.tag_id', 't.id') .where({ 'ta.assigned_id': assignedId, 'ta.assigned_to': assignedTo }) .pluck('t.name'); } +async function updateMentorTags(mentorId, tags = []) { + const knex = getKnex(); + const existing = await getAssignedTagNames(mentorId, 'mentor'); + + const toRemove = existing.filter((name) => !tags.includes(name)); + const toAdd = tags.filter((name) => !existing.includes(name)); + + if (toRemove.length) { + await knex('tags_assigned') + .where({ assigned_id: mentorId, assigned_to: 'mentor' }) + .whereIn( + 'tag_id', + (qb) => qb + .select('id') + .from('tags') + .whereIn('name', toRemove) + ) + .del(); + } + + for (const name of toAdd) { + const tag = await ensureTag(name); + await assignTagTo(tag.id, mentorId, 'mentor'); + } + + return mentorId; +} + module.exports = { getAllTags, - findTagByName, + getTagByName, createTag, ensureTag, assignTagTo, - removeAssignments, getAssignedTagNames, + updateMentorTags }; \ No newline at end of file diff --git a/routes/mentors.js b/routes/mentors.js index f05ed38..e56c1fd 100644 --- a/routes/mentors.js +++ b/routes/mentors.js @@ -108,6 +108,9 @@ router.put("/mentors/:id", verifyRoles([ROLE_NAMES.mentor]), verifyOwnership(OWN if (!isValidIntegerId(id)) { return respondWithError(res, 400, "Invalid user id supplied"); } + if (!Object.keys(updates).length) { + return respondWithError(res, 400, "Updated fields are required"); + } // check that only fields 'about' and 'contact' can be updated const invalidFields = Object.keys(updates) diff --git a/services/mentor_service.js b/services/mentor_service.js index d17071e..653a68a 100644 --- a/services/mentor_service.js +++ b/services/mentor_service.js @@ -1,4 +1,5 @@ const repo = require('../repositories/mentor_repository'); +const tagsRepo = require('../repositories/tags_repository'); const rolesService = require('../services/roles_service'); const mailerService = require('../services/mailer_service'); const { MentorAlreadyExistsError, DataRequiresElevatedRoleError } = require("../errors"); @@ -39,7 +40,9 @@ async function createMentor(userId, mentorData) { contact: mentorData.contact, tags: mentorData.tags }; + const mentorInfo = await repo.createMentor(mentor); + await tagsRepo.updateMentorTags(mentorInfo.id, mentorData.tags); const adminEmails = await rolesService.getAdminEmails(); await mailerService.sendEmailOnNewMentorApplication(adminEmails, mentorInfo); @@ -64,7 +67,9 @@ async function getMentors(userId, status, includeAdditionalFields) { async function getMentorById(userRoles, mentorId, isOwner) { const allowedStatuses = await getEligibleMentorStatuses(userRoles, isOwner); - return repo.getMentorById(allowedStatuses, allFields, mentorId); + const mentor = await repo.getMentorById(allowedStatuses, allFields, mentorId); + mentor.tags = await tagsRepo.getAssignedTagNames(mentorId, 'mentor'); + return mentor; } async function getEligibleMentorStatuses(userRoles, isOwner) { @@ -113,11 +118,19 @@ async function updateMentorStatus(userRoles, mentorId, status, isOwner) { async function updateMentor(userRoles, mentorId, updates) { const mentorData = await getMentorById(userRoles, mentorId); - if(mentorData.status === MENTOR_STATUSES.active || mentorData.status === MENTOR_STATUSES.inactive) { - const updatedMentorId = await repo.updateMentorById(mentorId, updates); - return updatedMentorId; + if (![MENTOR_STATUSES.active, MENTOR_STATUSES.inactive].includes(mentorData.status)) { + return 0; + } + let updatedMentorId; + const { tags, ...mentorUpdates } = updates; + if(tags) { + await tagsRepo.updateMentorTags(mentorId, tags); + } + if(Object.keys(mentorUpdates).length) { + updatedMentorId = await repo.updateMentorById(mentorId, mentorUpdates); } - return 0; + + return updatedMentorId || parseInt(mentorId, 10); } async function deleteMentorById(mentorId, userId) { @@ -126,6 +139,8 @@ async function deleteMentorById(mentorId, userId) { } const deletedMentorId = await repo.deleteMentorById(mentorId); removeRoleFromUser(userId, 2); + // remove associated tags + await tagsRepo.updateMentorTags(mentorId, []); return deletedMentorId; } diff --git a/tests/services/mentor_service.test.js b/tests/services/mentor_service.test.js index 6e5908b..d53ba90 100644 --- a/tests/services/mentor_service.test.js +++ b/tests/services/mentor_service.test.js @@ -4,10 +4,12 @@ const rolesService = require('../../services/roles_service'); const mailerService = require('../../services/mailer_service'); const repo = require('../../repositories/mentor_repository'); const rolesRepo = require('../../repositories/roles_repository'); +const tagsRepo = require('../../repositories/tags_repository'); const { MentorAlreadyExistsError, DataRequiresElevatedRoleError } = require('../../errors'); jest.mock('../../repositories/mentor_repository'); jest.mock('../../repositories/roles_repository'); +jest.mock('../../repositories/tags_repository'); jest.mock('../../services/roles_service'); jest.mock('../../services/mailer_service'); @@ -23,7 +25,7 @@ describe('Mentor Service', () => { describe('createMentor', () => { it('Create mentor in pending state success', async () => { const mentorData = { about: 'I am a mentor', contact: 'mentor@example.com', tags: mockedTags }; - const createdMentor = { id: 1, name: 'Name', about: mentorData.about }; + const createdMentor = { id: 1, name: 'Name', about: mentorData.about, tags: mentorData.tags }; repo.getMentorByUserId.mockResolvedValue(null); repo.createMentor.mockResolvedValue(createdMentor); @@ -37,6 +39,7 @@ describe('Mentor Service', () => { contact: mentorData.contact, tags: mentorData.tags }); + expect(tagsRepo.updateMentorTags).toHaveBeenCalledWith(createdMentor.id, mentorData.tags); expect(rolesService.getAdminEmails).toHaveBeenCalled(); expect(mailerService.sendEmailOnNewMentorApplication).toHaveBeenCalled(); expect(result).toEqual(createdMentor.id); @@ -345,6 +348,29 @@ describe('Mentor Service', () => { expect(result).toBe(mockedMentorId); }); + it('Successful update of tags field', async () => { + repo.getMentorById.mockResolvedValue({ + id: mockedMentorId, + user_id: defaultUserId, + status: MENTOR_STATUSES.inactive, + }); + repo.updateMentorById.mockResolvedValue(mockedMentorId); + + const result = await mentorService.updateMentor( + [{ role_name: 'mentor' }], + mockedMentorId.toString(), + { tags: mockedTags }, + true // isOwner + ); + + expect(repo.updateMentorById).not.toHaveBeenCalled(); + expect(tagsRepo.updateMentorTags).toHaveBeenCalledWith( + mockedMentorId.toString(), + mockedTags + ); + expect(result).toBe(mockedMentorId); + }); + it('Failed update if mentor status is not active or inactive', async () => { repo.getMentorById.mockResolvedValue({ id: mockedMentorId, From fc830a27204728e406df219f1c21b83f8d9f676a Mon Sep 17 00:00:00 2001 From: Marina Kim Date: Sat, 1 Nov 2025 13:07:24 +0000 Subject: [PATCH 3/3] returns tags in getMentors --- services/mentor_service.js | 15 +++++++++++++-- tests/services/mentor_service.test.js | 13 ++++++++++--- 2 files changed, 23 insertions(+), 5 deletions(-) diff --git a/services/mentor_service.js b/services/mentor_service.js index 653a68a..2ed7cc4 100644 --- a/services/mentor_service.js +++ b/services/mentor_service.js @@ -61,8 +61,19 @@ async function getMentors(userId, status, includeAdditionalFields) { throw new DataRequiresElevatedRoleError('Requested data requires elevated role'); } const statusesFilter = status ? [status] : allowedStatuses; - - return await repo.getMentors(statusesFilter, selectedFields); + const mentors = await repo.getMentors(statusesFilter, selectedFields); + + const mentorsWithTags = await Promise.all( + mentors.map(async (mentor) => { + const tags = await tagsRepo.getAssignedTagNames(mentor.mentor_id, 'mentor'); + return { + ...mentor, + tags + }; + }) + ); + + return mentorsWithTags; } async function getMentorById(userRoles, mentorId, isOwner) { diff --git a/tests/services/mentor_service.test.js b/tests/services/mentor_service.test.js index d53ba90..dc23dc6 100644 --- a/tests/services/mentor_service.test.js +++ b/tests/services/mentor_service.test.js @@ -62,17 +62,24 @@ describe('Mentor Service', () => { describe('getMentors', () => { it('Unauthenticated users see active and inactive mentors with limited fields', async () => { const allowedStatuses = mentorService.generalVisitbleStatuses; - const mentor = { id: 1, user_id: defaultUserId, status: MENTOR_STATUSES.active }; + const mentor = { mentor_id: 1, user_id: defaultUserId, status: MENTOR_STATUSES.active }; rolesService.getUserRoles.mockResolvedValue(); repo.getMentors.mockResolvedValue([mentor]); - - await mentorService.getMentors(undefined, undefined, false); + tagsRepo.getAssignedTagNames.mockResolvedValue(mockedTags); + const result = await mentorService.getMentors(undefined, undefined, false); expect(allowedStatuses).toContain(MENTOR_STATUSES.active); expect(allowedStatuses).toContain(MENTOR_STATUSES.inactive); expect(rolesService.getUserRoles).not.toHaveBeenCalled(); expect(repo.getMentors).toHaveBeenCalledWith(allowedStatuses, mentorService.baseFields); + expect(tagsRepo.getAssignedTagNames).toHaveBeenCalledWith(mentor.mentor_id, 'mentor'); + expect(result).toEqual([ + { + ...mentor, + tags: mockedTags + } + ]); }); it('Unauthenticated users cannot see non-active mentors', async () => {