From 43699a21ceb6f9b49a05432737633293bf6db101 Mon Sep 17 00:00:00 2001 From: Angelo Teixeira Date: Mon, 28 Dec 2020 22:27:11 +0000 Subject: [PATCH 1/3] Added search index and modified get offers endpoint --- src/models/Offer.js | 5 +++++ src/services/offer.js | 7 +++++-- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/src/models/Offer.js b/src/models/Offer.js index 569745db..73dc2fa3 100644 --- a/src/models/Offer.js +++ b/src/models/Offer.js @@ -81,6 +81,11 @@ const OfferSchema = new Schema({ coordinates: { type: PointSchema, required: false }, }); +OfferSchema.index( + { title: "text", jobType: "text", fields: "text", technologies: "text", location: "text" }, + { name: "Search index", weights: { title: 10, jobType: 5, location: 5, fields: 5, technologies: 5 } } +); + OfferSchema.methods.withCompany = async function() { const offer = this.toObject(); diff --git a/src/services/offer.js b/src/services/offer.js index 2cd0ffb3..b0cb7023 100644 --- a/src/services/offer.js +++ b/src/services/offer.js @@ -52,13 +52,16 @@ class OfferService { return offer; } - async get({ offset = 0, limit = OfferService.MAX_OFFERS_PER_QUERY, showHidden = false }) { + async get({ value = "", offset = 0, limit = OfferService.MAX_OFFERS_PER_QUERY, showHidden = false }) { - const offers = Offer.find().current(); + const offers = value ? Offer.find( + { "$text": { "$search": value } }, { score: { "$meta": "textScore" } } + ) : Offer.find({}, { score: { "$meta": "textScore" } }).current(); if (!showHidden) offers.withoutHidden(); return Promise.all((await offers + .sort({ score: { "$meta": "textScore" } }) .skip(offset) .limit(limit) ).map((o) => o.withCompany())); From a9d31c4fafb791678af27b294200bb7502951233 Mon Sep 17 00:00:00 2001 From: Angelo Teixeira Date: Tue, 29 Dec 2020 15:08:18 +0000 Subject: [PATCH 2/3] Added search filters and fixed field labels --- src/api/middleware/validators/application.js | 5 +- src/api/middleware/validators/offer.js | 45 ++- .../middleware/validators/validatorUtils.js | 7 + src/models/Offer.js | 3 +- src/models/constants/FieldTypes.js | 4 +- src/models/constants/JobTypes.js | 4 +- src/services/offer.js | 30 +- test/end-to-end/offer.js | 329 ++++++++++++++++-- test/end-to-end/review.js | 9 +- 9 files changed, 379 insertions(+), 57 deletions(-) diff --git a/src/api/middleware/validators/application.js b/src/api/middleware/validators/application.js index a76ab2a1..43d725fb 100644 --- a/src/api/middleware/validators/application.js +++ b/src/api/middleware/validators/application.js @@ -2,7 +2,7 @@ const { body, param, query } = require("express-validator"); const { useExpressValidators } = require("../errorHandler"); const ValidationReasons = require("./validationReasons"); -const { checkDuplicatedEmail, valuesInSet } = require("./validatorUtils"); +const { checkDuplicatedEmail, valuesInSet, ensureArray } = require("./validatorUtils"); const CompanyApplicationConstants = require("../../../models/constants/CompanyApplication"); const CompanyConstants = require("../../../models/constants/Company"); const AccountConstants = require("../../../models/constants/Account"); @@ -101,8 +101,7 @@ const search = useExpressValidators([ .isString().withMessage(ValidationReasons.STRING), query("state", ValidationReasons.DEFAULT) .optional() - .isJSON().withMessage(ValidationReasons.ARRAY).bail() - .customSanitizer(JSON.parse) + .customSanitizer(ensureArray) .isArray().withMessage(ValidationReasons.ARRAY).bail() .custom(valuesInSet(Object.keys(ApplicationStatus))), query("submissionDateFrom", ValidationReasons.DEFAULT) diff --git a/src/api/middleware/validators/offer.js b/src/api/middleware/validators/offer.js index f3c25b23..38bae20c 100644 --- a/src/api/middleware/validators/offer.js +++ b/src/api/middleware/validators/offer.js @@ -2,10 +2,10 @@ const { body, query } = require("express-validator"); const { useExpressValidators } = require("../errorHandler"); const ValidationReasons = require("./validationReasons"); -const { valuesInSet } = require("./validatorUtils"); +const { valuesInSet, ensureArray } = require("./validatorUtils"); const JobTypes = require("../../../models/constants/JobTypes"); -const FieldTypes = require("../../../models/constants/FieldTypes"); -const TechnologyTypes = require("../../../models/constants/TechnologyTypes"); +const { FieldTypes, MIN_FIELDS, MAX_FIELDS } = require("../../../models/constants/FieldTypes"); +const { TechnologyTypes, MIN_TECHNOLOGIES, MAX_TECHNOLOGIES } = require("../../../models/constants/TechnologyTypes"); const OfferService = require("../../../services/offer"); const OfferConstants = require("../../../models/constants/Offer"); const Company = require("../../../models/Company"); @@ -82,17 +82,17 @@ const create = useExpressValidators([ body("fields", ValidationReasons.DEFAULT) .exists().withMessage(ValidationReasons.REQUIRED).bail() - .isArray({ min: FieldTypes.MIN_FIELDS, max: FieldTypes.MAX_FIELDS }) - .withMessage(ValidationReasons.ARRAY_SIZE(FieldTypes.MIN_FIELDS, FieldTypes.MAX_FIELDS)) + .isArray({ min: MIN_FIELDS, max: MAX_FIELDS }) + .withMessage(ValidationReasons.ARRAY_SIZE(MIN_FIELDS, MAX_FIELDS)) .bail() - .custom(valuesInSet(FieldTypes.FieldTypes)), + .custom(valuesInSet(FieldTypes)), body("technologies", ValidationReasons.DEFAULT) .exists().withMessage(ValidationReasons.REQUIRED).bail() - .isArray({ min: TechnologyTypes.MIN_TECHNOLOGIES, max: TechnologyTypes.MAX_TECHNOLOGIES }) - .withMessage(ValidationReasons.ARRAY_SIZE(TechnologyTypes.MIN_TECHNOLOGIES, TechnologyTypes.MAX_TECHNOLOGIES)) + .isArray({ min: MIN_TECHNOLOGIES, max: MAX_TECHNOLOGIES }) + .withMessage(ValidationReasons.ARRAY_SIZE(MIN_TECHNOLOGIES, MAX_TECHNOLOGIES)) .bail() - .custom(valuesInSet(TechnologyTypes.TechnologyTypes)), + .custom(valuesInSet(TechnologyTypes)), body("isHidden", ValidationReasons.DEFAULT) .optional() @@ -144,6 +144,33 @@ const get = useExpressValidators([ .optional() .isBoolean().withMessage(ValidationReasons.BOOLEAN) .toBoolean(), + + query("jobType") + .optional() + .isString().withMessage(ValidationReasons.STRING).bail() + .isIn(JobTypes).withMessage(ValidationReasons.IN_ARRAY(JobTypes)), + + query("jobMinDuration", ValidationReasons.DEFAULT) + .optional() + .isInt().withMessage(ValidationReasons.INT).bail() + .toInt(), + + query("jobMaxDuration", ValidationReasons.DEFAULT) + .optional() + .isInt().withMessage(ValidationReasons.INT).bail() + .toInt(), + + query("fields", ValidationReasons.DEFAULT) + .optional() + .customSanitizer(ensureArray) + .isArray().withMessage(ValidationReasons.ARRAY).bail() + .custom(valuesInSet((FieldTypes))), + + query("technologies", ValidationReasons.DEFAULT) + .optional() + .customSanitizer(ensureArray) + .isArray().withMessage(ValidationReasons.ARRAY).bail() + .custom(valuesInSet((TechnologyTypes))), ]); module.exports = { create, get }; diff --git a/src/api/middleware/validators/validatorUtils.js b/src/api/middleware/validators/validatorUtils.js index 052e251e..ed7c5252 100644 --- a/src/api/middleware/validators/validatorUtils.js +++ b/src/api/middleware/validators/validatorUtils.js @@ -26,7 +26,14 @@ const checkDuplicatedEmail = async (email) => { } }; +const ensureArray = (val) => { + if (Array.isArray(val)) return val; + + else return [val]; +}; + module.exports = { valuesInSet, checkDuplicatedEmail, + ensureArray }; diff --git a/src/models/Offer.js b/src/models/Offer.js index 73dc2fa3..56f86594 100644 --- a/src/models/Offer.js +++ b/src/models/Offer.js @@ -76,6 +76,7 @@ const OfferSchema = new Schema({ default: false }, owner: { type: Types.ObjectId, ref: "Company", required: true }, + // ownerName: { type: String, required: true }, location: { type: String, required: true }, coordinates: { type: PointSchema, required: false }, @@ -89,7 +90,7 @@ OfferSchema.index( OfferSchema.methods.withCompany = async function() { const offer = this.toObject(); - const company = await Company.findById(offer.owner); + const company = (await Company.findById(offer.owner)).toObject(); return { ...offer, company }; }; diff --git a/src/models/constants/FieldTypes.js b/src/models/constants/FieldTypes.js index 38b555f9..3f56270a 100644 --- a/src/models/constants/FieldTypes.js +++ b/src/models/constants/FieldTypes.js @@ -1,6 +1,6 @@ const FieldTypes = Object.freeze([ - "BACK_END", - "FRONT_END", + "BACKEND", + "FRONTEND", "DEVOPS", "BLOCKCHAIN", "MACHINE LEARNING", diff --git a/src/models/constants/JobTypes.js b/src/models/constants/JobTypes.js index 36c17262..2fd2185e 100644 --- a/src/models/constants/JobTypes.js +++ b/src/models/constants/JobTypes.js @@ -1,6 +1,6 @@ const JobTypes = Object.freeze([ - "FULL_TIME", - "PART_TIME", + "FULL-TIME", + "PART-TIME", "SUMMER INTERNSHIP", "CURRICULAR INTERNSHIP", "OTHER", diff --git a/src/services/offer.js b/src/services/offer.js index b0cb7023..7dcce970 100644 --- a/src/services/offer.js +++ b/src/services/offer.js @@ -52,21 +52,43 @@ class OfferService { return offer; } - async get({ value = "", offset = 0, limit = OfferService.MAX_OFFERS_PER_QUERY, showHidden = false }) { + /** + * Fetches offers according to specified options + * @param {*} options + * value: Text to use in full-text-search + * offset: Point to start looking (and limiting) + * limit: How many offers to show + * jobType: Array of jobTypes allowed + */ + async get({ value = "", offset = 0, limit = OfferService.MAX_OFFERS_PER_QUERY, showHidden = false, ...filters }) { const offers = value ? Offer.find( - { "$text": { "$search": value } }, { score: { "$meta": "textScore" } } - ) : Offer.find({}, { score: { "$meta": "textScore" } }).current(); + { "$and": [this._buildFilterQuery(filters), { "$text": { "$search": value } }] }, { score: { "$meta": "textScore" } } + ) : Offer.find(this._buildFilterQuery(filters)).current(); if (!showHidden) offers.withoutHidden(); return Promise.all((await offers - .sort({ score: { "$meta": "textScore" } }) + .sort(value ? { score: { "$meta": "textScore" } } : undefined) .skip(offset) .limit(limit) ).map((o) => o.withCompany())); } + _buildFilterQuery(filters) { + if (!filters || !Object.keys(filters).length) return {}; + + const { jobType, jobMinDuration, jobMaxDuration, fields, technologies } = filters; + const constraints = []; + + if (jobType) constraints.push({ jobType: { "$in": jobType } }); + if (jobMinDuration) constraints.push({ jobMinDuration: { "$gte": jobMinDuration } }); + if (jobMaxDuration) constraints.push({ jobMaxDuration: { "$lte": jobMaxDuration } }); + if (fields?.length) constraints.push({ fields: { "$elemMatch": { "$in": fields } } }); + if (technologies?.length) constraints.push({ technologies: { "$elemMatch": { "$in": technologies } } }); + + return constraints.length ? { "$and": constraints } : {}; + } } module.exports = OfferService; diff --git a/test/end-to-end/offer.js b/test/end-to-end/offer.js index a986a3cd..04f8f2af 100644 --- a/test/end-to-end/offer.js +++ b/test/end-to-end/offer.js @@ -343,12 +343,47 @@ describe("Offer endpoint tests", () => { const FieldValidatorTester = QueryValidatorTester("limit"); FieldValidatorTester.mustBeNumber(); }); + + describe("jobMinDuration", () => { + const FieldValidatorTester = QueryValidatorTester("jobMinDuration"); + FieldValidatorTester.mustBeNumber(); + }); + + describe("jobMaxDuration", () => { + const FieldValidatorTester = QueryValidatorTester("jobMaxDuration"); + FieldValidatorTester.mustBeNumber(); + }); + + describe("jobType", () => { + const FieldValidatorTester = QueryValidatorTester("jobType"); + FieldValidatorTester.mustBeInArray(JobTypes); + }); + + describe("fields", () => { + const FieldValidatorTester = QueryValidatorTester("fields"); + FieldValidatorTester.mustHaveValuesInRange(FieldTypes.FieldTypes, FieldTypes.MIN_FIELDS + 1); + }); + + describe("technologies", () => { + const FieldValidatorTester = QueryValidatorTester("technologies"); + FieldValidatorTester.mustHaveValuesInRange(TechnologyTypes.TechnologyTypes, TechnologyTypes.MIN_TECHNOLOGIES + 1); + }); }); describe("Using already created offer(s)", () => { - describe("Only current offers are returned", () => { + let test_company; + let test_offer; - const test_offer = { + beforeAll(async () => { + + await Company.deleteMany({}); + test_company = await Company.create({ + name: "test company", + bio: "a bio", + contacts: ["a contact"] + }); + + test_offer = { title: "Test Offer", publishDate: "2019-11-22T00:00:00.000Z", publishEndDate: "2019-11-28T00:00:00.000Z", @@ -357,9 +392,27 @@ describe("Offer endpoint tests", () => { jobType: "SUMMER INTERNSHIP", fields: ["DEVOPS", "MACHINE LEARNING", "OTHER"], technologies: ["React", "CSS"], - // owner: Will be set in beforeAll, + owner: test_company._id, location: "Testing Street, Test City, 123", }; + + await Offer.deleteMany({}); + await Offer.create(test_offer); + }); + + const RealDateNow = Date.now; + const mockCurrentDate = new Date("2019-11-23"); + + beforeEach(() => { + Date.now = () => mockCurrentDate.getTime(); + }); + + afterEach(() => { + Date.now = RealDateNow; + }); + + describe("Only current offers are returned", () => { + const expired_test_offer = { title: "Expired Test Offer", publishDate: "2019-11-17", @@ -369,7 +422,7 @@ describe("Offer endpoint tests", () => { jobType: "SUMMER INTERNSHIP", fields: ["DEVOPS", "MACHINE LEARNING", "OTHER"], technologies: ["React", "CSS"], - // owner: Will be set in beforeAll, + // owner: Will be set in beforeAll, since it is not accessible here location: "Testing Street, Test City, 123", }; const future_test_offer = { @@ -381,43 +434,20 @@ describe("Offer endpoint tests", () => { jobType: "SUMMER INTERNSHIP", fields: ["DEVOPS", "MACHINE LEARNING", "OTHER"], technologies: ["React", "CSS"], - // owner: Will be set in beforeAll, + // owner: Will be set in beforeAll, since it is not accessible here location: "Testing Street, Test City, 123", }; - let test_company; beforeAll(async () => { - await Company.deleteMany({}); - test_company = await Company.create({ - name: "test company", - bio: "a bio", - contacts: ["a contact"] - }); - - [test_offer, future_test_offer, expired_test_offer] + [future_test_offer, expired_test_offer] .forEach((offer) => { offer.owner = test_company._id; }); - await Offer.deleteMany({}); - await Offer.create([test_offer, expired_test_offer, future_test_offer]); + await Offer.create([expired_test_offer, future_test_offer]); }); - afterAll(async () => { - await Offer.deleteMany({}); - }); - - const RealDateNow = Date.now; - const mockCurrentDate = new Date("2019-11-23"); - - beforeEach(() => { - Date.now = () => mockCurrentDate.getTime(); - }); - - afterEach(() => { - Date.now = RealDateNow; - }); test("should provide only current offer info (no expired or future offers)", async () => { const res = await request() @@ -427,7 +457,7 @@ describe("Offer endpoint tests", () => { expect(res.body).toHaveLength(1); // Necessary because jest matchers appear to not be working (expect.any(Number), expect.anthing(), etc) const extracted_data = res.body.map((elem) => { - delete elem["_id"]; delete elem["__v"]; delete elem["owner"]; return elem; + delete elem["_id"]; delete elem["__v"]; delete elem["owner"]; delete elem["score"]; return elem; }); const prepared_test_offer = { ...test_offer, @@ -600,6 +630,243 @@ describe("Offer endpoint tests", () => { expect(extracted_data).toContainEqual(prepared_test_offer); }); }); + + }); + + describe("Full text search", () => { + + let portoFrontend; + let portoBackend; + let lisboaBackend; + + beforeAll(async () => { + portoFrontend = { + ...test_offer, + location: "Porto", + jobType: "FULL-TIME", + fields: ["FRONTEND", "OTHER"], + jobMinDuration: 3, + jobMaxDuration: 6 + }; + portoBackend = { + ...test_offer, + location: "Porto", + fields: ["BACKEND", "OTHER"], + jobMinDuration: 2, + jobMaxDuration: 4 + }; + lisboaBackend = { + ...test_offer, + location: "Lisboa", + fields: ["BACKEND", "DEVOPS"] + }; + await Offer.deleteMany({}); + await Offer.create([portoBackend, portoFrontend, lisboaBackend]); + }); + + test("should return porto offers", async () => { + + const res = await request() + .get("/offers") + .query({ + value: "porto" + }); + + expect(res.status).toBe(HTTPStatus.OK); + expect(res.body).toHaveLength(2); + + // Necessary because jest matchers appear to not be working (expect.any(Number), expect.anthing(), etc) + const extracted_data = res.body.map((elem) => { + delete elem["_id"]; delete elem["__v"]; delete elem["owner"]; delete elem["score"]; + return elem; + }); + + // eslint-disable-next-line no-unused-vars + const expected_offers = [portoBackend, portoFrontend].map(({ owner, ...offer }) => ({ + ...offer, + isHidden: false, + company: JSON.parse(JSON.stringify(test_company.toObject())) + })); + + expected_offers.forEach((expected) => { + expect(extracted_data).toContainEqual(expected); + }); + }); + + test("should return porto offers in order", async () => { + + const res = await request() + .get("/offers") + .query({ + value: "porto frontend" + }); + + expect(res.status).toBe(HTTPStatus.OK); + expect(res.body).toHaveLength(2); + + // Necessary because jest matchers appear to not be working (expect.any(Number), expect.anthing(), etc) + const extracted_data = res.body.map((elem) => { + delete elem["_id"]; delete elem["__v"]; delete elem["owner"]; delete elem["score"]; + return elem; + }); + + // eslint-disable-next-line no-unused-vars + const expected_offers = [portoFrontend, portoBackend].map(({ owner, ...offer }) => ({ + ...offer, + isHidden: false, + company: JSON.parse(JSON.stringify(test_company.toObject())) + })); + + expected_offers.forEach((expected, i) => { + expect(extracted_data[i]).toEqual(expected); + }); + }); + + test("should return porto offers for FULL-TIME", async () => { + + const res = await request() + .get("/offers") + .query({ + value: "porto", + jobType: "FULL-TIME" + }); + + expect(res.status).toBe(HTTPStatus.OK); + expect(res.body).toHaveLength(1); + + // Necessary because jest matchers appear to not be working (expect.any(Number), expect.anthing(), etc) + const extracted_data = res.body.map((elem) => { + delete elem["_id"]; delete elem["__v"]; delete elem["owner"]; delete elem["score"]; + return elem; + }); + + const prepared_test_offer = { + ...portoFrontend, + isHidden: false, + company: JSON.parse(JSON.stringify(test_company.toObject())) + }; + + delete prepared_test_offer["owner"]; + + expect(extracted_data).toContainEqual(prepared_test_offer); + }); + + test("should return porto offers with React", async () => { + + const res = await request() + .get("/offers") + .query({ + value: "porto", + technologies: ["React"] + }); + + expect(res.status).toBe(HTTPStatus.OK); + expect(res.body).toHaveLength(2); + + // Necessary because jest matchers appear to not be working (expect.any(Number), expect.anthing(), etc) + const extracted_data = res.body.map((elem) => { + delete elem["_id"]; delete elem["__v"]; delete elem["owner"]; delete elem["score"]; + return elem; + }); + + // eslint-disable-next-line no-unused-vars + const expected_offers = [portoFrontend, portoBackend].map(({ owner, ...offer }) => ({ + ...offer, + isHidden: false, + company: JSON.parse(JSON.stringify(test_company.toObject())) + })); + + expected_offers.forEach((expected) => { + expect(extracted_data).toContainEqual(expected); + }); + }); + + test("should return offers with DEVOPS", async () => { + + const res = await request() + .get("/offers") + .query({ + fields: ["DEVOPS"] + }); + + expect(res.status).toBe(HTTPStatus.OK); + expect(res.body).toHaveLength(1); + + // Necessary because jest matchers appear to not be working (expect.any(Number), expect.anthing(), etc) + const extracted_data = res.body.map((elem) => { + delete elem["_id"]; delete elem["__v"]; delete elem["owner"]; delete elem["score"]; + return elem; + }); + + const prepared_test_offer = { + ...lisboaBackend, + isHidden: false, + company: JSON.parse(JSON.stringify(test_company.toObject())) + }; + + delete prepared_test_offer["owner"]; + + expect(extracted_data).toContainEqual(prepared_test_offer); + }); + + test("should return porto offers with min duration of 2", async () => { + + const res = await request() + .get("/offers") + .query({ + value: "porto", + jobMinDuration: 2 + }); + + expect(res.status).toBe(HTTPStatus.OK); + expect(res.body).toHaveLength(2); + + // Necessary because jest matchers appear to not be working (expect.any(Number), expect.anthing(), etc) + const extracted_data = res.body.map((elem) => { + delete elem["_id"]; delete elem["__v"]; delete elem["owner"]; delete elem["score"]; + return elem; + }); + + // eslint-disable-next-line no-unused-vars + const expected_offers = [portoFrontend, portoBackend].map(({ owner, ...offer }) => ({ + ...offer, + isHidden: false, + company: JSON.parse(JSON.stringify(test_company.toObject())) + })); + + expected_offers.forEach((expected) => { + expect(extracted_data).toContainEqual(expected); + }); + }); + + test("should return porto offers with min duration of 2 and max duration of 4", async () => { + + const res = await request() + .get("/offers") + .query({ + value: "porto", + jobMinDuration: 2, + jobMaxDuration: 4 + }); + expect(res.status).toBe(HTTPStatus.OK); + expect(res.body).toHaveLength(1); + + // Necessary because jest matchers appear to not be working (expect.any(Number), expect.anthing(), etc) + const extracted_data = res.body.map((elem) => { + delete elem["_id"]; delete elem["__v"]; delete elem["owner"]; delete elem["score"]; + return elem; + }); + + const prepared_test_offer = { + ...portoBackend, + isHidden: false, + company: JSON.parse(JSON.stringify(test_company.toObject())) + }; + + delete prepared_test_offer["owner"]; + + expect(extracted_data).toContainEqual(prepared_test_offer); + }); }); }); }); diff --git a/test/end-to-end/review.js b/test/end-to-end/review.js index 4b1f5972..c311daaa 100644 --- a/test/end-to-end/review.js +++ b/test/end-to-end/review.js @@ -133,28 +133,27 @@ describe("Company application review endpoint test", () => { test("Should filter by state", async () => { - const wrongFormatQuery = await test_agent .get(`/applications/company/search?state=<["${APPROVED}"]`); expect(wrongFormatQuery.status).toBe(HTTPStatus.UNPROCESSABLE_ENTITY); expect(wrongFormatQuery.body.errors[0]).toStrictEqual({ location: "query", - msg: "must-be-array", + msg: "must-be-in:[PENDING,APPROVED,REJECTED]", param: "state", - value: `<["${APPROVED}"]` + value: [`<["${APPROVED}"]`] }); const singleStateQuery = await test_agent - .get(`/applications/company/search?state=["${APPROVED}"]`); + .get("/applications/company/search").query({ state: [APPROVED] }); expect(singleStateQuery.status).toBe(HTTPStatus.OK); expect(singleStateQuery.body.applications.length).toBe(1); expect(singleStateQuery.body.applications[0]).toHaveProperty("companyName", approvedApplication.companyName); const multiStateQuery = await test_agent - .get(`/applications/company/search?state=["${APPROVED}", "${PENDING}"]`); + .get("/applications/company/search?").query({ state: [APPROVED, PENDING] }); expect(multiStateQuery.status).toBe(HTTPStatus.OK); expect(multiStateQuery.body.applications.length).toBe(2); From 627e8b14650e14207669aec35729712569bfdcc2 Mon Sep 17 00:00:00 2001 From: Angelo Teixeira Date: Tue, 5 Jan 2021 00:03:15 +0000 Subject: [PATCH 3/3] Added Full-text-search on ownerName for offers --- .../middleware/validators/validatorUtils.js | 8 ++ src/loaders/mongoose.js | 1 + src/models/Company.js | 13 +++ src/models/Offer.js | 15 +-- src/services/offer.js | 10 +- test/company_schema.js | 48 ++++++++ test/end-to-end/offer.js | 107 +++++++++++------- test/offer_schema.js | 12 ++ 8 files changed, 157 insertions(+), 57 deletions(-) diff --git a/src/api/middleware/validators/validatorUtils.js b/src/api/middleware/validators/validatorUtils.js index ed7c5252..0e3cc447 100644 --- a/src/api/middleware/validators/validatorUtils.js +++ b/src/api/middleware/validators/validatorUtils.js @@ -26,6 +26,14 @@ const checkDuplicatedEmail = async (email) => { } }; +/** + * Sanitizes the input val to return an array. If val is an array, this is a no-op + * Otherwise wraps val in an array + * + * This is especially helpful when you expect an array in a query param, + * but a one-element array is given, therefore it is parsed as a string instead + * @param {*} val + */ const ensureArray = (val) => { if (Array.isArray(val)) return val; diff --git a/src/loaders/mongoose.js b/src/loaders/mongoose.js index 61f3b9ab..2bf3f244 100644 --- a/src/loaders/mongoose.js +++ b/src/loaders/mongoose.js @@ -15,6 +15,7 @@ const setupDbConnection = async () => { dbName: config.db_name, useNewUrlParser: true, useCreateIndex: true, + useFindAndModify: false }; try { diff --git a/src/models/Company.js b/src/models/Company.js index dece1875..15c4e078 100644 --- a/src/models/Company.js +++ b/src/models/Company.js @@ -1,6 +1,7 @@ const mongoose = require("mongoose"); const { Schema } = mongoose; const CompanyConstants = require("./constants/Company"); +const Offer = require("./Offer"); const CompanySchema = new Schema({ name: { @@ -18,6 +19,18 @@ const CompanySchema = new Schema({ }, }); +// Update offers from this company on name change +CompanySchema.post("findOneAndUpdate", async function(doc) { + await Offer.updateMany({ owner: doc._id }, { ownerName: doc.name }); +}); + +// Delete Offers from the deleted Company (maybe we want to archive them instead?, +// also maybe we want other hooks as well such as deleteOne) +// CompanySchema.post("findOneAndDelete", async function(doc) { +// await Offer.deleteMany({ owner: doc._id }, { ownerName: doc.name }); +// }); + const Company = mongoose.model("Company", CompanySchema); + module.exports = Company; diff --git a/src/models/Offer.js b/src/models/Offer.js index 56f86594..8284dca9 100644 --- a/src/models/Offer.js +++ b/src/models/Offer.js @@ -5,7 +5,6 @@ const JobTypes = require("./constants/JobTypes"); const { FieldTypes, MIN_FIELDS, MAX_FIELDS } = require("./constants/FieldTypes"); const { TechnologyTypes, MIN_TECHNOLOGIES, MAX_TECHNOLOGIES } = require("./constants/TechnologyTypes"); const PointSchema = require("./Point"); -const Company = require("./Company"); const { MONTH_IN_MS, OFFER_MAX_LIFETIME_MONTHS } = require("./constants/TimeConstants"); const { noDuplicatesValidator, lengthBetweenValidator } = require("./modelUtils"); const OfferConstants = require("./constants/Offer"); @@ -76,25 +75,17 @@ const OfferSchema = new Schema({ default: false }, owner: { type: Types.ObjectId, ref: "Company", required: true }, - // ownerName: { type: String, required: true }, + ownerName: { type: String, required: true }, location: { type: String, required: true }, coordinates: { type: PointSchema, required: false }, }); OfferSchema.index( - { title: "text", jobType: "text", fields: "text", technologies: "text", location: "text" }, - { name: "Search index", weights: { title: 10, jobType: 5, location: 5, fields: 5, technologies: 5 } } + { title: "text", ownerName: "text", jobType: "text", fields: "text", technologies: "text", location: "text" }, + { name: "Search index", weights: { title: 10, ownerName: 5, jobType: 5, location: 5, fields: 5, technologies: 5 } } ); -OfferSchema.methods.withCompany = async function() { - const offer = this.toObject(); - - const company = (await Company.findById(offer.owner)).toObject(); - - return { ...offer, company }; -}; - // Checking if the publication date is less than or equal than the end date. function validatePublishDate(value) { return value <= this.publishEndDate; diff --git a/src/services/offer.js b/src/services/offer.js index 7dcce970..442a653d 100644 --- a/src/services/offer.js +++ b/src/services/offer.js @@ -1,3 +1,4 @@ +const Company = require("../models/Company"); const Offer = require("../models/Offer"); class OfferService { @@ -29,6 +30,8 @@ class OfferService { location, coordinates, }) { + + const ownerName = (await Company.findById(owner)).name; const offer = await Offer.create({ title, publishDate, @@ -45,6 +48,7 @@ class OfferService { technologies, isHidden, owner, + ownerName, location, coordinates, }); @@ -60,7 +64,7 @@ class OfferService { * limit: How many offers to show * jobType: Array of jobTypes allowed */ - async get({ value = "", offset = 0, limit = OfferService.MAX_OFFERS_PER_QUERY, showHidden = false, ...filters }) { + get({ value = "", offset = 0, limit = OfferService.MAX_OFFERS_PER_QUERY, showHidden = false, ...filters }) { const offers = value ? Offer.find( { "$and": [this._buildFilterQuery(filters), { "$text": { "$search": value } }] }, { score: { "$meta": "textScore" } } @@ -68,11 +72,11 @@ class OfferService { if (!showHidden) offers.withoutHidden(); - return Promise.all((await offers + return offers .sort(value ? { score: { "$meta": "textScore" } } : undefined) .skip(offset) .limit(limit) - ).map((o) => o.withCompany())); + ; } _buildFilterQuery(filters) { diff --git a/test/company_schema.js b/test/company_schema.js index ec14f3ae..f38194a8 100644 --- a/test/company_schema.js +++ b/test/company_schema.js @@ -1,6 +1,8 @@ const Company = require("../src/models/Company"); const SchemaTester = require("./utils/SchemaTester"); const CompanyConstants = require("../src/models/constants/Company"); +const { DAY_TO_MS } = require("./utils/TimeConstants"); +const Offer = require("../src/models/Offer"); const CompanySchemaTester = SchemaTester(Company); @@ -16,4 +18,50 @@ describe("# Company Schema tests", () => { CompanySchemaTester.maxLength("name", CompanyConstants.companyName.max_length); }); }); + + describe("Hook tests", () => { + + describe("Company name update", () => { + + let company; + + beforeAll(async () => { + company = await Company.create({ + name: "first name" + }); + const offer = { + title: "Test Offer", + publishDate: new Date(Date.now() - (DAY_TO_MS)), + publishEndDate: new Date(Date.now() + (DAY_TO_MS)), + description: "For Testing Purposes", + contacts: ["geral@niaefeup.pt", "229417766"], + jobType: "SUMMER INTERNSHIP", + fields: ["DEVOPS", "MACHINE LEARNING", "OTHER"], + technologies: ["React", "CSS"], + location: "Testing Street, Test City, 123", + owner: company._id, + ownerName: company.name + }; + + await Offer.create([offer, offer]); + }); + + afterAll(async () => { + await Offer.deleteMany({}); + await Company.deleteMany({}); + }); + + test("Should update offers ownerName on company name update", async () => { + const offersBefore = await Offer.find({ owner: company._id }); + + expect(offersBefore.every(({ ownerName }) => ownerName === "first name")).toBe(true); + + await Company.findByIdAndUpdate(company._id, { name: "new name" }, { new: true }); + + const offersAfter = await Offer.find({ owner: company._id }); + + expect(offersAfter.every(({ ownerName }) => ownerName === "new name")).toBe(true); + }); + }); + }); }); diff --git a/test/end-to-end/offer.js b/test/end-to-end/offer.js index 04f8f2af..9a337602 100644 --- a/test/end-to-end/offer.js +++ b/test/end-to-end/offer.js @@ -88,7 +88,7 @@ describe("Offer endpoint tests", () => { .send(test_user_admin) .expect(200); - const res = await request() + const res = await test_agent .post("/offers/new") .send(offer); @@ -277,6 +277,7 @@ describe("Offer endpoint tests", () => { const offer_params = { ...offer, owner: test_company._id, + ownerName: test_company.name }; const res = await request() @@ -308,6 +309,7 @@ describe("Offer endpoint tests", () => { fields: ["DEVOPS", "MACHINE LEARNING", "OTHER"], technologies: ["React", "CSS"], owner: test_company._id, + ownerName: test_company.name, location: "Testing Street, Test City, 123", }; @@ -393,6 +395,7 @@ describe("Offer endpoint tests", () => { fields: ["DEVOPS", "MACHINE LEARNING", "OTHER"], technologies: ["React", "CSS"], owner: test_company._id, + ownerName: test_company.name, location: "Testing Street, Test City, 123", }; @@ -423,6 +426,7 @@ describe("Offer endpoint tests", () => { fields: ["DEVOPS", "MACHINE LEARNING", "OTHER"], technologies: ["React", "CSS"], // owner: Will be set in beforeAll, since it is not accessible here + // ownerName: Will be set in beforeAll, since it is not accessible here location: "Testing Street, Test City, 123", }; const future_test_offer = { @@ -435,6 +439,7 @@ describe("Offer endpoint tests", () => { fields: ["DEVOPS", "MACHINE LEARNING", "OTHER"], technologies: ["React", "CSS"], // owner: Will be set in beforeAll, since it is not accessible here + // ownerName: Will be set in beforeAll, since it is not accessible here location: "Testing Street, Test City, 123", }; @@ -443,6 +448,7 @@ describe("Offer endpoint tests", () => { [future_test_offer, expired_test_offer] .forEach((offer) => { offer.owner = test_company._id; + offer.ownerName = test_company.name; }); await Offer.create([expired_test_offer, future_test_offer]); @@ -457,15 +463,13 @@ describe("Offer endpoint tests", () => { expect(res.body).toHaveLength(1); // Necessary because jest matchers appear to not be working (expect.any(Number), expect.anthing(), etc) const extracted_data = res.body.map((elem) => { - delete elem["_id"]; delete elem["__v"]; delete elem["owner"]; delete elem["score"]; return elem; + delete elem["_id"]; delete elem["__v"]; delete elem["score"]; return elem; }); const prepared_test_offer = { ...test_offer, isHidden: false, - // JSON.parse->JSON.stringify needed because comparison below fails otherwise. Spread operator does not work - company: JSON.parse(JSON.stringify(test_company.toObject())) + owner: test_offer.owner.toString() }; - delete prepared_test_offer["owner"]; expect(extracted_data).toContainEqual(prepared_test_offer); }); @@ -489,18 +493,16 @@ describe("Offer endpoint tests", () => { // Necessary because jest matchers appear to not be working (expect.any(Number), expect.anthing(), etc) const extracted_data = res.body.map((elem) => { - delete elem["_id"]; delete elem["__v"]; delete elem["owner"]; + delete elem["_id"]; delete elem["__v"]; return elem; }); const prepared_test_offer = { ...test_offer, isHidden: false, - company: JSON.parse(JSON.stringify(test_company.toObject())) + owner: test_offer.owner.toString() }; - delete prepared_test_offer["owner"]; - expect(extracted_data).toContainEqual(prepared_test_offer); }); }); @@ -547,18 +549,16 @@ describe("Offer endpoint tests", () => { // Necessary because jest matchers appear to not be working (expect.any(Number), expect.anthing(), etc) const extracted_data = res.body.map((elem) => { - delete elem["_id"]; delete elem["__v"]; delete elem["owner"]; + delete elem["_id"]; delete elem["__v"]; return elem; }); const prepared_test_offer = { ...test_offer, isHidden: false, - company: JSON.parse(JSON.stringify(test_company.toObject())) + owner: test_offer.owner.toString() }; - delete prepared_test_offer["owner"]; - expect(extracted_data).toContainEqual(prepared_test_offer); }); @@ -581,18 +581,16 @@ describe("Offer endpoint tests", () => { // Necessary because jest matchers appear to not be working (expect.any(Number), expect.anthing(), etc) const extracted_data = res.body.map((elem) => { - delete elem["_id"]; delete elem["__v"]; delete elem["owner"]; + delete elem["_id"]; delete elem["__v"]; return elem; }); const prepared_test_offer = { ...test_offer, isHidden: false, - company: JSON.parse(JSON.stringify(test_company.toObject())) + owner: test_offer.owner.toString() }; - delete prepared_test_offer["owner"]; - expect(extracted_data).toContainEqual(prepared_test_offer); }); @@ -615,18 +613,16 @@ describe("Offer endpoint tests", () => { // Necessary because jest matchers appear to not be working (expect.any(Number), expect.anthing(), etc) const extracted_data = res.body.map((elem) => { - delete elem["_id"]; delete elem["__v"]; delete elem["owner"]; + delete elem["_id"]; delete elem["__v"]; return elem; }); const prepared_test_offer = { ...test_offer, isHidden: false, - company: JSON.parse(JSON.stringify(test_company.toObject())) + owner: test_offer.owner.toString() }; - delete prepared_test_offer["owner"]; - expect(extracted_data).toContainEqual(prepared_test_offer); }); }); @@ -638,6 +634,7 @@ describe("Offer endpoint tests", () => { let portoFrontend; let portoBackend; let lisboaBackend; + let niaefeupOffer; beforeAll(async () => { portoFrontend = { @@ -660,8 +657,14 @@ describe("Offer endpoint tests", () => { location: "Lisboa", fields: ["BACKEND", "DEVOPS"] }; + niaefeupOffer = { + ...test_offer, + location: "FEUP", + fields: ["BLOCKCHAIN", "OTHER"], + ownerName: "NIAEFEUP" + }; await Offer.deleteMany({}); - await Offer.create([portoBackend, portoFrontend, lisboaBackend]); + await Offer.create([portoBackend, portoFrontend, lisboaBackend, niaefeupOffer]); }); test("should return porto offers", async () => { @@ -677,7 +680,7 @@ describe("Offer endpoint tests", () => { // Necessary because jest matchers appear to not be working (expect.any(Number), expect.anthing(), etc) const extracted_data = res.body.map((elem) => { - delete elem["_id"]; delete elem["__v"]; delete elem["owner"]; delete elem["score"]; + delete elem["_id"]; delete elem["__v"]; delete elem["score"]; return elem; }); @@ -685,7 +688,7 @@ describe("Offer endpoint tests", () => { const expected_offers = [portoBackend, portoFrontend].map(({ owner, ...offer }) => ({ ...offer, isHidden: false, - company: JSON.parse(JSON.stringify(test_company.toObject())) + owner: owner.toString() })); expected_offers.forEach((expected) => { @@ -693,6 +696,32 @@ describe("Offer endpoint tests", () => { }); }); + test("should return niaefeup (company) offers", async () => { + + const res = await request() + .get("/offers") + .query({ + value: "niaefeup" + }); + + expect(res.status).toBe(HTTPStatus.OK); + expect(res.body).toHaveLength(1); + + // Necessary because jest matchers appear to not be working (expect.any(Number), expect.anthing(), etc) + const extracted_data = res.body.map((elem) => { + delete elem["_id"]; delete elem["__v"]; delete elem["score"]; + return elem; + }); + + const prepared_test_offer = { + ...niaefeupOffer, + isHidden: false, + owner: niaefeupOffer.owner.toString() + }; + + expect(extracted_data).toContainEqual(prepared_test_offer); + }); + test("should return porto offers in order", async () => { const res = await request() @@ -706,7 +735,7 @@ describe("Offer endpoint tests", () => { // Necessary because jest matchers appear to not be working (expect.any(Number), expect.anthing(), etc) const extracted_data = res.body.map((elem) => { - delete elem["_id"]; delete elem["__v"]; delete elem["owner"]; delete elem["score"]; + delete elem["_id"]; delete elem["__v"]; delete elem["score"]; return elem; }); @@ -714,7 +743,7 @@ describe("Offer endpoint tests", () => { const expected_offers = [portoFrontend, portoBackend].map(({ owner, ...offer }) => ({ ...offer, isHidden: false, - company: JSON.parse(JSON.stringify(test_company.toObject())) + owner: owner.toString() })); expected_offers.forEach((expected, i) => { @@ -736,18 +765,16 @@ describe("Offer endpoint tests", () => { // Necessary because jest matchers appear to not be working (expect.any(Number), expect.anthing(), etc) const extracted_data = res.body.map((elem) => { - delete elem["_id"]; delete elem["__v"]; delete elem["owner"]; delete elem["score"]; + delete elem["_id"]; delete elem["__v"]; delete elem["score"]; return elem; }); const prepared_test_offer = { ...portoFrontend, isHidden: false, - company: JSON.parse(JSON.stringify(test_company.toObject())) + owner: portoFrontend.owner.toString() }; - delete prepared_test_offer["owner"]; - expect(extracted_data).toContainEqual(prepared_test_offer); }); @@ -765,7 +792,7 @@ describe("Offer endpoint tests", () => { // Necessary because jest matchers appear to not be working (expect.any(Number), expect.anthing(), etc) const extracted_data = res.body.map((elem) => { - delete elem["_id"]; delete elem["__v"]; delete elem["owner"]; delete elem["score"]; + delete elem["_id"]; delete elem["__v"]; delete elem["score"]; return elem; }); @@ -773,7 +800,7 @@ describe("Offer endpoint tests", () => { const expected_offers = [portoFrontend, portoBackend].map(({ owner, ...offer }) => ({ ...offer, isHidden: false, - company: JSON.parse(JSON.stringify(test_company.toObject())) + owner: owner.toString() })); expected_offers.forEach((expected) => { @@ -794,18 +821,16 @@ describe("Offer endpoint tests", () => { // Necessary because jest matchers appear to not be working (expect.any(Number), expect.anthing(), etc) const extracted_data = res.body.map((elem) => { - delete elem["_id"]; delete elem["__v"]; delete elem["owner"]; delete elem["score"]; + delete elem["_id"]; delete elem["__v"]; delete elem["score"]; return elem; }); const prepared_test_offer = { ...lisboaBackend, isHidden: false, - company: JSON.parse(JSON.stringify(test_company.toObject())) + owner: lisboaBackend.owner.toString() }; - delete prepared_test_offer["owner"]; - expect(extracted_data).toContainEqual(prepared_test_offer); }); @@ -823,7 +848,7 @@ describe("Offer endpoint tests", () => { // Necessary because jest matchers appear to not be working (expect.any(Number), expect.anthing(), etc) const extracted_data = res.body.map((elem) => { - delete elem["_id"]; delete elem["__v"]; delete elem["owner"]; delete elem["score"]; + delete elem["_id"]; delete elem["__v"]; delete elem["score"]; return elem; }); @@ -831,7 +856,7 @@ describe("Offer endpoint tests", () => { const expected_offers = [portoFrontend, portoBackend].map(({ owner, ...offer }) => ({ ...offer, isHidden: false, - company: JSON.parse(JSON.stringify(test_company.toObject())) + owner: owner.toString() })); expected_offers.forEach((expected) => { @@ -853,18 +878,16 @@ describe("Offer endpoint tests", () => { // Necessary because jest matchers appear to not be working (expect.any(Number), expect.anthing(), etc) const extracted_data = res.body.map((elem) => { - delete elem["_id"]; delete elem["__v"]; delete elem["owner"]; delete elem["score"]; + delete elem["_id"]; delete elem["__v"]; delete elem["score"]; return elem; }); const prepared_test_offer = { ...portoBackend, isHidden: false, - company: JSON.parse(JSON.stringify(test_company.toObject())) + owner: portoBackend.owner.toString() }; - delete prepared_test_offer["owner"]; - expect(extracted_data).toContainEqual(prepared_test_offer); }); }); diff --git a/test/offer_schema.js b/test/offer_schema.js index 7d00d1b8..1ed86007 100644 --- a/test/offer_schema.js +++ b/test/offer_schema.js @@ -91,6 +91,18 @@ describe("# Offer Schema tests", () => { } }); + test("'ownerName' is required", async () => { + const offer = new Offer({}); + // Returning the validation promise to ensure the test doesn't finish before all the assertions do + try { + await offer.validate(); + } catch (err) { + expect(err.errors.ownerName).toBeDefined(); + expect(err.errors.ownerName).toHaveProperty("kind", "required"); + expect(err.errors.ownerName).toHaveProperty("message", "Path `ownerName` is required."); + } + }); + test("'location' is required", async () => { const offer = new Offer({}); // Returning the validation promise to ensure the test doesn't finish before all the assertions do