diff --git a/.changeset/nasty-tools-enjoy.md b/.changeset/nasty-tools-enjoy.md new file mode 100644 index 000000000000..b6e8dae3785a --- /dev/null +++ b/.changeset/nasty-tools-enjoy.md @@ -0,0 +1,5 @@ +--- +"@rocket.chat/meteor": patch +--- + +Fixed a code issue on NPS service. It was passing `startAt` as the expiration date when creating a banner. diff --git a/apps/meteor/server/services/nps/service.ts b/apps/meteor/server/services/nps/service.ts index 9554a472fb0a..be5b44582a5a 100644 --- a/apps/meteor/server/services/nps/service.ts +++ b/apps/meteor/server/services/nps/service.ts @@ -21,7 +21,10 @@ export class NPSService extends ServiceClassInternal implements INPSService { const any = await Nps.findOne({}, { projection: { _id: 1 } }); if (!any) { - await Banner.create(getBannerForAdmins(nps.startAt)); + if (nps.expireAt < nps.startAt || nps.expireAt < new Date()) { + throw new Error('NPS already expired'); + } + await Banner.create(getBannerForAdmins(nps.expireAt)); await notifyAdmins(nps.startAt); } diff --git a/apps/meteor/tests/unit/server/services/nps/spec.tests.ts b/apps/meteor/tests/unit/server/services/nps/spec.tests.ts new file mode 100644 index 000000000000..21c5ecfe7c98 --- /dev/null +++ b/apps/meteor/tests/unit/server/services/nps/spec.tests.ts @@ -0,0 +1,103 @@ +import { expect } from 'chai'; +import { describe, it } from 'mocha'; +import p from 'proxyquire'; +import sinon from 'sinon'; + +const modelsMock = { + 'NpsVote': {}, + 'Nps': { + findOne: sinon.stub(), + save: sinon.stub(), + }, + 'Settings': { + getValueById: sinon.stub(), + }, + '@noCallThru': true, +}; + +const servicesMock = { + Banner: { + create: sinon.stub(), + }, +}; + +const getbannerforadminsMock = sinon.stub(); + +const { NPSService } = p('../../../../../server/services/nps/service.ts', { + '@rocket.chat/models': modelsMock, + '@rocket.chat/core-services': servicesMock, + './sendNpsResults': { 'sendNpsResults': sinon.stub(), '@noCallThru': true }, + '../../lib/logger/system': { 'SystemLogger': { error: sinon.stub() }, '@noCallThru': true }, + './notification': { 'notifyAdmins': sinon.stub(), 'getBannerForAdmins': getbannerforadminsMock, '@noCallThru': true }, +}); + +describe('NPS Service', () => { + it('should instantiate properly', () => { + expect(new NPSService()).to.be.an('object'); + }); + + describe('@create', () => { + beforeEach(() => { + modelsMock.Settings.getValueById.reset(); + modelsMock.Nps.findOne.reset(); + modelsMock.Nps.save.reset(); + servicesMock.Banner.create.reset(); + getbannerforadminsMock.reset(); + }); + it('should fail when user opted out of nps', async () => { + modelsMock.Settings.getValueById.withArgs('NPS_survey_enabled').resolves(false); + + await expect(new NPSService().create({})).to.be.rejectedWith('Server opted-out for NPS surveys'); + }); + it('should fail when nps expireDate is less than nps startAt', async () => { + modelsMock.Settings.getValueById.withArgs('NPS_survey_enabled').resolves(true); + modelsMock.Nps.findOne.resolves(null); + + await expect(new NPSService().create({ expireAt: new Date('2020-01-01'), startAt: new Date('2020-01-02') })).to.be.rejectedWith( + 'NPS already expired', + ); + }); + it('should fail when expireDate is less than current date', async () => { + modelsMock.Settings.getValueById.withArgs('NPS_survey_enabled').resolves(true); + modelsMock.Nps.findOne.resolves(null); + + await expect(new NPSService().create({ expireAt: new Date('2020-01-02'), startAt: new Date('2020-01-01') })).to.be.rejectedWith( + 'NPS already expired', + ); + }); + it('should try to create a banner when theres no nps saved', async () => { + modelsMock.Settings.getValueById.withArgs('NPS_survey_enabled').resolves(true); + modelsMock.Nps.findOne.resolves(null); + + const today = new Date(); + const tomorrow = new Date(today.getTime() + 24 * 60 * 60 * 1000); + + await new NPSService().create({ + expireAt: tomorrow, + startAt: today, + createdBy: { _id: 'tomorrow', username: 'tomorrow' }, + npsId: 'test', + }); + expect(getbannerforadminsMock.called).to.be.true; + expect(getbannerforadminsMock.calledWith(tomorrow)).to.be.true; + expect(modelsMock.Nps.save.called).to.be.true; + expect( + modelsMock.Nps.save.calledWith( + sinon.match({ + expireAt: tomorrow, + startAt: today, + status: 'open', + _id: 'test', + createdBy: { _id: 'tomorrow', username: 'tomorrow' }, + }), + ), + ).to.be.true; + }); + it('should fail if theres an error when saving the Nps', async () => { + modelsMock.Settings.getValueById.withArgs('NPS_survey_enabled').resolves(true); + modelsMock.Nps.findOne.resolves({ _id: 'test' }); + modelsMock.Nps.save.rejects(); + await expect(new NPSService().create({})).to.be.rejectedWith('Error creating NPS'); + }); + }); +});