From 36979b0c823823803f025ca20df895558fdec2dc Mon Sep 17 00:00:00 2001 From: WikiRik Date: Sun, 21 Mar 2021 20:41:07 +0000 Subject: [PATCH 1/4] feat(campaign): various updates --- lib/constants.js | 11 +++++++++++ lib/helpers.js | 11 +++++++++++ lib/server.js | 4 +++- middlewares/campaigns.js | 12 ++++++++---- middlewares/fetch.js | 10 ++++++---- ...205034-change-campaign-short-description-null.js | 13 +++++++++++++ models/Campaign.js | 7 +++---- 7 files changed, 55 insertions(+), 13 deletions(-) create mode 100644 migrations/20210321205034-change-campaign-short-description-null.js diff --git a/lib/constants.js b/lib/constants.js index 6df7e082..59eab157 100644 --- a/lib/constants.js +++ b/lib/constants.js @@ -66,6 +66,17 @@ module.exports = { type: 'array' } }, + PUBLIC_FIELDS: { + CAMPAIGN: [ + 'id', + 'url', + 'name', + 'active', + 'description_short', + 'description_long', + 'autojoin_body' + ] + }, TOKEN_LENGTH: { MAIL_CONFIRMATION: 128, ACCESS_TOKEN: 32, diff --git a/lib/helpers.js b/lib/helpers.js index 48736a5f..6e63f82f 100644 --- a/lib/helpers.js +++ b/lib/helpers.js @@ -161,6 +161,16 @@ function findBy(query, fields) { return whereObject; } +// A helper to whilelist object's properties. +function whitelistObject(object, allowedFields) { + const newObject = {}; + for (const field of allowedFields) { + newObject[field] = object[field]; + } + + return newObject; +} + function getRandomBytes(length) { return new Promise((resolve, reject) => { crypto.randomBytes(length / 2, (err, res) => { @@ -197,6 +207,7 @@ module.exports = { getSorting, filterBy, findBy, + whitelistObject, getRandomBytes, addGaugeData }; diff --git a/lib/server.js b/lib/server.js index 737b2940..e045ab8d 100644 --- a/lib/server.js +++ b/lib/server.js @@ -156,7 +156,9 @@ PermissionsRouter.put('/', permissions.updatePermission); PermissionsRouter.delete('/', permissions.deletePermission); // Everything related to a specific campaign. Auth only. -CampaignsRouter.use(middlewares.maybeAuthorize, middlewares.ensureAuthorized, fetch.fetchCampaign); +CampaignsRouter.use(middlewares.maybeAuthorize, fetch.fetchCampaign); +CampaignsRouter.get('/', campaigns.getCampaign); +CampaignsRouter.use(middlewares.ensureAuthorized); CampaignsRouter.get('/members', campaigns.listCampaignMembers); CampaignsRouter.get('/', campaigns.getCampaign); CampaignsRouter.put('/', campaigns.updateCampaign); diff --git a/middlewares/campaigns.js b/middlewares/campaigns.js index 20edc42d..d4c3fbe7 100644 --- a/middlewares/campaigns.js +++ b/middlewares/campaigns.js @@ -80,8 +80,12 @@ exports.listAllCampaigns = async (req, res) => { }; exports.getCampaign = async (req, res) => { - if (!req.permissions.hasPermission('global:view:campaign')) { - return errors.makeForbiddenError(res, 'Permission global:view:campaign is required, but not present.'); + if (!req.user || !req.permissions.hasPermission('global:create:campaign')) { + req.currentCampaign = helpers.whitelistObject(req.currentCampaign, constants.PUBLIC_FIELDS.CAMPAIGN); + } + + if (req.currentCampaign.autojoin_body) { + req.currentCampaign.autojoin_body = helpers.whitelistObject(req.currentCampaign.autojoin_body, ['id', 'name', 'email', 'phone', 'address', 'website']); } return res.json({ @@ -96,10 +100,10 @@ exports.createCampaign = async (req, res) => { } // TODO: filter out fields that are changed in the other way - const circle = await Campaign.create(req.body); + const campaign = await Campaign.create(req.body); return res.json({ success: true, - data: circle + data: campaign }); }; diff --git a/middlewares/fetch.js b/middlewares/fetch.js index fb4cdd54..4c7c77a7 100644 --- a/middlewares/fetch.js +++ b/middlewares/fetch.js @@ -123,13 +123,15 @@ exports.fetchPermission = async (req, res, next) => { }; exports.fetchCampaign = async (req, res, next) => { - // searching the campaign by id if it's numeric - if (!helpers.isNumber(req.params.campaign_id)) { - return errors.makeBadRequestError(res, 'Campaign ID is invalid.'); + // Checking if the passed ID is a string or not. + // If it is a string, find the campaign by URL, if not, find it by ID or URL. + let findObject = { url: req.params.campaign_id }; + if (!Number.isNaN(Number(req.params.campaign_id))) { + findObject = { id: Number(req.params.campaign_id) }; } const campaign = await Campaign.findOne({ - where: { id: Number(req.params.campaign_id) }, + where: findObject, include: [{ model: Body, as: 'autojoin_body' }] }); if (!campaign) { diff --git a/migrations/20210321205034-change-campaign-short-description-null.js b/migrations/20210321205034-change-campaign-short-description-null.js new file mode 100644 index 00000000..87488953 --- /dev/null +++ b/migrations/20210321205034-change-campaign-short-description-null.js @@ -0,0 +1,13 @@ +module.exports = { + up: (queryInterface, Sequelize) => queryInterface.changeColumn( + 'campaigns', + 'description_short', + Sequelize.TEXT + ), + down: (queryInterface, Sequelize) => queryInterface.changeColumn( + 'campaigns', + 'description_short', + Sequelize.TEXT, + { allowNull: false } + ), +}; diff --git a/models/Campaign.js b/models/Campaign.js index 733bcdf4..a9cf11a9 100644 --- a/models/Campaign.js +++ b/models/Campaign.js @@ -25,10 +25,9 @@ const Campaign = sequelize.define('campaign', { }, description_short: { type: Sequelize.TEXT, - allowNull: false, + allowNull: true, validate: { - notEmpty: { msg: 'Description should be set.' }, - notNull: { msg: 'Description should be set.' } + notEmpty: { msg: 'Description should be set.' } } }, description_long: { @@ -41,7 +40,7 @@ const Campaign = sequelize.define('campaign', { }, activate_user: { type: Sequelize.BOOLEAN, - allowNull: false, + allowNull: true, defaultValue: true } }, { From 93062bb764e22db5b49c7dbfba76fcf76a947f70 Mon Sep 17 00:00:00 2001 From: WikiRik Date: Sun, 21 Mar 2021 21:08:10 +0000 Subject: [PATCH 2/4] test(campaigns): fix test --- test/api/campaigns-details.test.js | 35 ++++++++++++++++++++++++------ test/unit/campaigns.test.js | 28 ------------------------ 2 files changed, 28 insertions(+), 35 deletions(-) diff --git a/test/api/campaigns-details.test.js b/test/api/campaigns-details.test.js index e1895297..d036520a 100644 --- a/test/api/campaigns-details.test.js +++ b/test/api/campaigns-details.test.js @@ -15,7 +15,7 @@ describe('Campaign details', () => { await generator.clearAll(); }); - test('should return 404 if the campaign is not found', async () => { + test('should return 404 if the campaign id is not found', async () => { const user = await generator.createUser({ superadmin: true }); const token = await generator.createAccessToken({}, user); @@ -33,7 +33,7 @@ describe('Campaign details', () => { expect(res.body).toHaveProperty('message'); }); - test('should return 400 if id is not a number', async () => { + test('should return 404 if the campaign name is not found', async () => { const user = await generator.createUser({ superadmin: true }); const token = await generator.createAccessToken({}, user); @@ -45,13 +45,13 @@ describe('Campaign details', () => { headers: { 'X-Auth-Token': token.value } }); - expect(res.statusCode).toEqual(400); + expect(res.statusCode).toEqual(404); expect(res.body.success).toEqual(false); expect(res.body).toHaveProperty('message'); expect(res.body).not.toHaveProperty('data'); }); - test('should fail if no permission', async () => { + test('should return less info if no permission', async () => { const user = await generator.createUser(); const token = await generator.createAccessToken({}, user); @@ -63,10 +63,10 @@ describe('Campaign details', () => { headers: { 'X-Auth-Token': token.value } }); - expect(res.statusCode).toEqual(403); + expect(res.statusCode).toEqual(200); expect(res.body.success).toEqual(false); - expect(res.body).not.toHaveProperty('data'); - expect(res.body).toHaveProperty('message'); + expect(res.body).toHaveProperty('data'); + expect(res.body.data).not.toHaveProperty('autojoin_body_id'); }); test('should find the campaign by id', async () => { @@ -89,4 +89,25 @@ describe('Campaign details', () => { expect(res.body).not.toHaveProperty('errors'); expect(res.body.data.id).toEqual(campaign.id); }); + + test('should find the campaign by name', async () => { + const user = await generator.createUser({ superadmin: true }); + const token = await generator.createAccessToken({}, user); + + await generator.createPermission({ scope: 'global', action: 'view', object: 'campaign' }); + + const campaign = await generator.createCampaign(); + + const res = await request({ + uri: '/campaigns/' + campaign.name, + method: 'GET', + headers: { 'X-Auth-Token': token.value } + }); + + expect(res.statusCode).toEqual(200); + expect(res.body.success).toEqual(true); + expect(res.body).toHaveProperty('data'); + expect(res.body).not.toHaveProperty('errors'); + expect(res.body.data.name).toEqual(campaign.name); + }); }); diff --git a/test/unit/campaigns.test.js b/test/unit/campaigns.test.js index 08589165..b142c616 100644 --- a/test/unit/campaigns.test.js +++ b/test/unit/campaigns.test.js @@ -67,32 +67,6 @@ describe('Campaigns testing', () => { } }); - test('should fail with null description_short', async () => { - try { - await generator.createCampaign({ description_short: null }); - expect(1).toEqual(0); - } catch (err) { - expect(err).toHaveProperty('errors'); - expect(err.errors.length).toEqual(1); - expect(err.errors[0].path).toEqual('description_short'); - } - }); - - test('should fail with not set description_short', async () => { - try { - const campaign = generator.generateCampaign(); - delete campaign.description_short; - - await Campaign.create(campaign); - - expect(1).toEqual(0); - } catch (err) { - expect(err).toHaveProperty('errors'); - expect(err.errors.length).toEqual(1); - expect(err.errors[0].path).toEqual('description_short'); - } - }); - test('should fail with null description_long', async () => { try { await generator.createCampaign({ description_long: null }); @@ -123,14 +97,12 @@ describe('Campaigns testing', () => { const data = generator.generateCampaign({ name: '\t\t\ttest\t\t\t', url: '\t\t\tTEST\t\t\t', - description_short: ' \t test\t \t', description_long: ' \t test\t \t', }); const campaign = await Campaign.create(data); expect(campaign.name).toEqual('test'); expect(campaign.url).toEqual('test'); - expect(campaign.description_short).toEqual('test'); expect(campaign.description_long).toEqual('test'); }); }); From 954d6de6659e2de924e418d7e4ea90b713180a40 Mon Sep 17 00:00:00 2001 From: WikiRik Date: Sun, 21 Mar 2021 21:41:02 +0000 Subject: [PATCH 3/4] test(campaigns): fix last tests --- test/api/campaigns-details.test.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/api/campaigns-details.test.js b/test/api/campaigns-details.test.js index d036520a..9b0b5d2c 100644 --- a/test/api/campaigns-details.test.js +++ b/test/api/campaigns-details.test.js @@ -64,7 +64,7 @@ describe('Campaign details', () => { }); expect(res.statusCode).toEqual(200); - expect(res.body.success).toEqual(false); + expect(res.body.success).toEqual(true); expect(res.body).toHaveProperty('data'); expect(res.body.data).not.toHaveProperty('autojoin_body_id'); }); @@ -90,7 +90,7 @@ describe('Campaign details', () => { expect(res.body.data.id).toEqual(campaign.id); }); - test('should find the campaign by name', async () => { + test('should find the campaign by url', async () => { const user = await generator.createUser({ superadmin: true }); const token = await generator.createAccessToken({}, user); @@ -99,7 +99,7 @@ describe('Campaign details', () => { const campaign = await generator.createCampaign(); const res = await request({ - uri: '/campaigns/' + campaign.name, + uri: '/campaigns/' + campaign.url, method: 'GET', headers: { 'X-Auth-Token': token.value } }); @@ -108,6 +108,6 @@ describe('Campaign details', () => { expect(res.body.success).toEqual(true); expect(res.body).toHaveProperty('data'); expect(res.body).not.toHaveProperty('errors'); - expect(res.body.data.name).toEqual(campaign.name); + expect(res.body.data.url).toEqual(campaign.url); }); }); From a45a8b4426d1ad0b3853e96b40a8d3ae0606865e Mon Sep 17 00:00:00 2001 From: WikiRik Date: Mon, 22 Mar 2021 14:04:42 +0000 Subject: [PATCH 4/4] test(campaigns): add test autojoin_body info --- test/api/campaigns-details.test.js | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/test/api/campaigns-details.test.js b/test/api/campaigns-details.test.js index 9b0b5d2c..33e9e973 100644 --- a/test/api/campaigns-details.test.js +++ b/test/api/campaigns-details.test.js @@ -69,6 +69,27 @@ describe('Campaign details', () => { expect(res.body.data).not.toHaveProperty('autojoin_body_id'); }); + test('should return some info about the autojoin_body', async () => { + const user = await generator.createUser(); + const token = await generator.createAccessToken({}, user); + + const body = await generator.createBody(); + const campaign = await generator.createCampaign({ autojoin_body_id: body.id }); + + const res = await request({ + uri: '/campaigns/' + campaign.id, + method: 'GET', + headers: { 'X-Auth-Token': token.value } + }); + + expect(res.statusCode).toEqual(200); + expect(res.body.success).toEqual(true); + expect(res.body).toHaveProperty('data'); + expect(res.body.data).toHaveProperty('autojoin_body'); + expect(res.body.data.autojoin_body).toHaveProperty('email'); + expect(res.body.data.autojoin_body).not.toHaveProperty('founded_at'); + }); + test('should find the campaign by id', async () => { const user = await generator.createUser({ superadmin: true }); const token = await generator.createAccessToken({}, user);