diff --git a/repositories/mentor_repository.js b/repositories/mentor_repository.js index a5caec4..905dc46 100644 --- a/repositories/mentor_repository.js +++ b/repositories/mentor_repository.js @@ -19,31 +19,36 @@ 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(); + + return mentor; } async function createMentor(mentorData) { const knex = getKnex(); - const [createdMentor] = await knex("mentors") - .insert(mentorData) - .returning("id"); + return await knex.transaction(async (trx) => { + const [createdMentor] = await trx("mentors") + .insert(mentorData) + .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(); + 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 updateMentorById(mentorId, updates) { const knex = getKnex(); + const [mentor] = await knex("mentors") .where("id", mentorId) .update(updates) diff --git a/repositories/tags_repository.js b/repositories/tags_repository.js index 6af13b5..5e6300f 100644 --- a/repositories/tags_repository.js +++ b/repositories/tags_repository.js @@ -6,6 +6,77 @@ async function getAllTags() { return names; } +async function getTagByName(name) { + const knex = getKnex(); + return await knex('tags') + .where('name', name) + .first(); +} + +async function createTag(name) { + const knex = getKnex(); + const [{ id }] = await knex('tags') + .insert({ name }) + .returning('id'); + return { id, name }; +} + +async function ensureTag(name) { + let tag = await getTagByName(name); + if (!tag) { + tag = await createTag(name); + } + return tag; +} + +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 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, + getTagByName, + createTag, + ensureTag, + assignTagTo, + getAssignedTagNames, + updateMentorTags }; \ No newline at end of file diff --git a/routes/mentors.js b/routes/mentors.js index d8aa5aa..e56c1fd 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); @@ -107,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 3cecfdc..2ed7cc4 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"); @@ -37,8 +38,11 @@ async function createMentor(userId, mentorData) { status: 'pending', about: mentorData.about, 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); @@ -57,13 +61,26 @@ 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) { 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) { @@ -112,11 +129,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) { @@ -125,6 +150,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 4dea1c5..dc23dc6 100644 --- a/tests/services/mentor_service.test.js +++ b/tests/services/mentor_service.test.js @@ -4,15 +4,18 @@ 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'); const defaultUserId = 42; const mockedMentorId = 1; +const mockedTags = ["React", "PHP"]; describe('Mentor Service', () => { afterEach(() => { @@ -21,8 +24,8 @@ 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 createdMentor = { id: 1, name: 'Name', about: mentorData.about }; + const mentorData = { about: 'I am a mentor', contact: 'mentor@example.com', tags: mockedTags }; + const createdMentor = { id: 1, name: 'Name', about: mentorData.about, tags: mentorData.tags }; repo.getMentorByUserId.mockResolvedValue(null); repo.createMentor.mockResolvedValue(createdMentor); @@ -34,7 +37,9 @@ describe('Mentor Service', () => { status: MENTOR_STATUSES.pending, about: mentorData.about, 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); @@ -57,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 () => { @@ -343,6 +355,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, 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,