From 7eb06d08587defc13b1f9c0aa31683f54bf10cd1 Mon Sep 17 00:00:00 2001 From: Pangratios Cosma Date: Wed, 27 Dec 2023 16:47:04 +0200 Subject: [PATCH] feat: make project-id redundant for check-ins configuration api --- packages/js/src/server.ts | 23 +-- .../src/server/check-ins-manager/check-in.ts | 53 +++--- .../js/src/server/check-ins-manager/client.ts | 84 ++++++--- .../js/src/server/check-ins-manager/index.ts | 46 ++--- .../js/src/server/check-ins-manager/types.ts | 15 +- packages/js/test/unit/server.test.ts | 44 +++-- .../test/unit/server/check-ins-sync.test.ts | 33 ++-- .../server/checkins-manager/checkin.test.ts | 33 ++-- .../server/checkins-manager/client.test.ts | 56 ++++-- .../server/checkins-manager/manager.test.ts | 168 +++++++++--------- 10 files changed, 319 insertions(+), 236 deletions(-) diff --git a/packages/js/src/server.ts b/packages/js/src/server.ts index 30e54796..3a11b075 100644 --- a/packages/js/src/server.ts +++ b/packages/js/src/server.ts @@ -92,9 +92,9 @@ class Honeybadger extends Client { throw new Error('Honeybadger.showUserFeedbackForm() is not supported on the server-side') } - async checkIn(idOrName: string): Promise { + async checkIn(idOrSlug: string): Promise { try { - const id = await this.getCheckInId(idOrName) + const id = await this.getCheckInId(idOrSlug) await this.__transport .send({ method: 'GET', @@ -104,30 +104,31 @@ class Honeybadger extends Client { this.logger.info('CheckIn sent') } catch (err) { - this.logger.error(`CheckIn[${idOrName}] failed: an unknown error occurred.`, `message=${err.message}`) + this.logger.error(`CheckIn[${idOrSlug}] failed: an unknown error occurred.`, `message=${err.message}`) } } - private async getCheckInId(idOrName: string): Promise { + private async getCheckInId(idOrSlug: string): Promise { if (!this.config.checkins || this.config.checkins.length === 0) { - return idOrName + return idOrSlug } - const localCheckIn = this.config.checkins.find(c => c.name === idOrName) + const localCheckIn = this.config.checkins.find(c => c.slug === idOrSlug) if (!localCheckIn) { - return idOrName + return idOrSlug } if (localCheckIn.id) { return localCheckIn.id } - const projectCheckIns = await this.__checkInsClient.listForProject(localCheckIn.projectId) - const remoteCheckIn = projectCheckIns.find(c => c.name === localCheckIn.name) + const projectId = await this.__checkInsClient.getProjectId(this.config.apiKey) + const projectCheckIns = await this.__checkInsClient.listForProject(projectId) + const remoteCheckIn = projectCheckIns.find(c => c.slug === localCheckIn.slug) if (!remoteCheckIn) { - this.logger.debug(`Checkin[${idOrName}] was not found on HB. This should not happen. Was the sync command executed?`) + this.logger.debug(`Checkin[${idOrSlug}] was not found on HB. This should not happen. Was the sync command executed?`) - return idOrName + return idOrSlug } // store the id in-memory, so subsequent check-ins won't have to call the API diff --git a/packages/js/src/server/check-ins-manager/check-in.ts b/packages/js/src/server/check-ins-manager/check-in.ts index 4cb4afee..86e2d286 100644 --- a/packages/js/src/server/check-ins-manager/check-in.ts +++ b/packages/js/src/server/check-ins-manager/check-in.ts @@ -2,10 +2,9 @@ import { CheckInDto, CheckInPayload, CheckInResponsePayload } from './types' export class CheckIn implements CheckInDto { id?: string - projectId: string - name: string + name?: string scheduleType: 'simple' | 'cron' - slug?: string + slug: string reportPeriod?: string gracePeriod?: string cronSchedule?: string @@ -27,7 +26,6 @@ export class CheckIn implements CheckInDto { this.gracePeriod = props.gracePeriod this.cronSchedule = props.cronSchedule this.cronTimezone = props.cronTimezone - this.projectId = props.projectId this.deleted = false } @@ -40,12 +38,8 @@ export class CheckIn implements CheckInDto { } public validate() { - if (!this.projectId) { - throw new Error('projectId is required for each check-in') - } - - if (!this.name) { - throw new Error('name is required for each check-in') + if (!this.slug) { + throw new Error('slug is required for each check-in') } if (!this.scheduleType) { @@ -53,23 +47,34 @@ export class CheckIn implements CheckInDto { } if (!['simple', 'cron'].includes(this.scheduleType)) { - throw new Error(`${this.name} [scheduleType] must be "simple" or "cron"`) + throw new Error(`${this.slug} [scheduleType] must be "simple" or "cron"`) } if (this.scheduleType === 'simple' && !this.reportPeriod) { - throw new Error(`${this.name} [reportPeriod] is required for simple check-ins`) + throw new Error(`${this.slug} [reportPeriod] is required for simple check-ins`) } if (this.scheduleType === 'cron' && !this.cronSchedule) { - throw new Error(`${this.name} [cronSchedule] is required for cron check-ins`) + throw new Error(`${this.slug} [cronSchedule] is required for cron check-ins`) } } + public update(other: CheckIn) { + this.id = other.id + this.slug = other.slug + this.name = other.name + this.scheduleType = other.scheduleType + this.reportPeriod = other.reportPeriod + this.gracePeriod = other.gracePeriod + this.cronSchedule = other.cronSchedule + this.cronTimezone = other.cronTimezone + } + public asRequestPayload() { const payload: CheckInPayload = { name: this.name, schedule_type: this.scheduleType, - slug: this.slug ?? '', // default is empty string + slug: this.slug, grace_period: this.gracePeriod ?? '' // default is empty string } @@ -88,21 +93,27 @@ export class CheckIn implements CheckInDto { * Compares two check-ins, usually the one from the API and the one from the config file. * If the one in the config file does not match the check-in from the API, * then we issue an update request. + * + * `name`, `gracePeriod` and `cronTimezone` are optional fields that are automatically + * set to a value from the server if one is not provided, + * so we ignore their values if they are not set locally. */ public isInSync(other: CheckIn) { - return this.name === other.name - && this.projectId === other.projectId + const ignoreNameCheck = this.name === undefined + const ignoreGracePeriodCheck = this.gracePeriod === undefined + const ignoreCronTimezoneCheck = this.cronTimezone === undefined + + return this.slug === other.slug && this.scheduleType === other.scheduleType && this.reportPeriod === other.reportPeriod && this.cronSchedule === other.cronSchedule - && (this.slug ?? '') === (other.slug ?? '') - && (this.gracePeriod ?? '') === (other.gracePeriod ?? '') - && (this.cronTimezone ?? '') === (other.cronTimezone ?? '') + && (ignoreNameCheck || this.name === other.name) + && (ignoreGracePeriodCheck || this.gracePeriod === other.gracePeriod) + && (ignoreCronTimezoneCheck || this.cronTimezone === other.cronTimezone) } - public static fromResponsePayload(projectId: string, payload: CheckInResponsePayload) { + public static fromResponsePayload(payload: CheckInResponsePayload) { return new CheckIn({ - projectId, id: payload.id, name: payload.name, slug: payload.slug, diff --git a/packages/js/src/server/check-ins-manager/client.ts b/packages/js/src/server/check-ins-manager/client.ts index cd8b0c53..498cf2b8 100644 --- a/packages/js/src/server/check-ins-manager/client.ts +++ b/packages/js/src/server/check-ins-manager/client.ts @@ -1,24 +1,53 @@ import { Types } from '@honeybadger-io/core' import { CheckIn } from './check-in' -import { CheckInResponsePayload } from './types'; +import { CheckInResponsePayload, CheckInsConfig } from './types'; export class CheckInsClient { private readonly BASE_URL = 'https://app.honeybadger.io' - private readonly cache: Record - private readonly config: { personalAuthToken: string; logger: Types.Logger } + private readonly config: Pick private readonly logger: Types.Logger private readonly transport: Types.Transport - constructor(config: { personalAuthToken: string; logger: Types.Logger }, transport: Types.Transport) { + private projectId: string | null + private cache: CheckIn[] | null + + constructor(config: Pick, transport: Types.Transport) { this.transport = transport this.config = config this.logger = config.logger - this.cache = {} + this.cache = null + this.projectId = null + } + + public async getProjectId(projectApiKey: string): Promise { + if (this.projectId) { + return this.projectId + } + + if (!this.config.personalAuthToken || this.config.personalAuthToken === '') { + throw new Error('personalAuthToken is required') + } + + const response = await this.transport.send({ + method: 'GET', + headers: this.getHeaders(), + endpoint: `${this.BASE_URL}/v2/project_keys/${projectApiKey}`, + logger: this.logger, + }) + + if (response.statusCode !== 200) { + throw new Error(`Failed to fetch project[${projectApiKey}]: ${this.getErrorMessage(response.body)}`) + } + + const data: { project: { id: string; name: string; created_at: string; } } = JSON.parse(response.body) + this.projectId = data?.project?.id + + return this.projectId } public async listForProject(projectId: string): Promise { - if (this.cache[projectId]) { - return this.cache[projectId] + if (this.cache !== null) { + return this.cache } if (!this.config.personalAuthToken || this.config.personalAuthToken === '') { @@ -37,10 +66,9 @@ export class CheckInsClient { } const data: { results: CheckInResponsePayload[] } = JSON.parse(response.body) - const checkIns = data.results.map((checkin) => CheckIn.fromResponsePayload(projectId, checkin)) - this.cache[projectId] = checkIns + this.cache = data.results.map((checkin) => CheckIn.fromResponsePayload(checkin)) - return checkIns + return this.cache } public async get(projectId: string, checkInId: string): Promise { @@ -60,13 +88,10 @@ export class CheckInsClient { } const data: CheckInResponsePayload = JSON.parse(response.body) - const checkIn = CheckIn.fromResponsePayload(projectId, data) - checkIn.projectId = projectId - - return checkIn + return CheckIn.fromResponsePayload(data) } - public async create(checkIn: CheckIn): Promise { + public async create(projectId: string, checkIn: CheckIn): Promise { if (!this.config.personalAuthToken || this.config.personalAuthToken === '') { throw new Error('personalAuthToken is required') } @@ -74,22 +99,22 @@ export class CheckInsClient { const response = await this.transport.send({ method: 'POST', headers: this.getHeaders(), - endpoint: `${this.BASE_URL}/v2/projects/${checkIn.projectId}/check_ins`, + endpoint: `${this.BASE_URL}/v2/projects/${projectId}/check_ins`, logger: this.logger, }, { check_in: checkIn.asRequestPayload() }) if (response.statusCode !== 201) { - throw new Error(`Failed to create check-in[${checkIn.name}] for project[${checkIn.projectId}]: ${this.getErrorMessage(response.body)}`) + throw new Error(`Failed to create check-in[${checkIn.slug}] for project[${projectId}]: ${this.getErrorMessage(response.body)}`) } const data: CheckInResponsePayload = JSON.parse(response.body) - const result = CheckIn.fromResponsePayload(checkIn.projectId, data) - result.projectId = checkIn.projectId + const checkin = CheckIn.fromResponsePayload(data) + this.cache?.push(checkin) - return result + return checkin } - public async update(checkIn: CheckIn): Promise { + public async update(projectId: string, checkIn: CheckIn): Promise { if (!this.config.personalAuthToken || this.config.personalAuthToken === '') { throw new Error('personalAuthToken is required') } @@ -97,18 +122,21 @@ export class CheckInsClient { const response = await this.transport.send({ method: 'PUT', headers: this.getHeaders(), - endpoint: `${this.BASE_URL}/v2/projects/${checkIn.projectId}/check_ins/${checkIn.id}`, + endpoint: `${this.BASE_URL}/v2/projects/${projectId}/check_ins/${checkIn.id}`, logger: this.logger, }, { check_in: checkIn.asRequestPayload() }) if (response.statusCode !== 204) { - throw new Error(`Failed to update checkin[${checkIn.name}] for project[${checkIn.projectId}]: ${this.getErrorMessage(response.body)}`) + throw new Error(`Failed to update checkin[${checkIn.slug}] for project[${projectId}]: ${this.getErrorMessage(response.body)}`) } + const cached = this.cache?.find(c => c.slug === checkIn.slug) + cached?.update(checkIn) + return checkIn } - public async remove(checkIn: CheckIn): Promise { + public async remove(projectId: string, checkIn: CheckIn): Promise { if (!this.config.personalAuthToken || this.config.personalAuthToken === '') { throw new Error('personalAuthToken is required') } @@ -116,12 +144,16 @@ export class CheckInsClient { const response = await this.transport.send({ method: 'DELETE', headers: this.getHeaders(), - endpoint: `${this.BASE_URL}/v2/projects/${checkIn.projectId}/check_ins/${checkIn.id}`, + endpoint: `${this.BASE_URL}/v2/projects/${projectId}/check_ins/${checkIn.id}`, logger: this.logger, }) if (response.statusCode !== 204) { - throw new Error(`Failed to remove checkin[${checkIn.name}] for project[${checkIn.projectId}]: ${this.getErrorMessage(response.body)}`) + throw new Error(`Failed to remove checkin[${checkIn.slug}] for project[${projectId}]: ${this.getErrorMessage(response.body)}`) + } + + if (this.cache) { + this.cache = this.cache.filter(c => c.slug !== checkIn.slug) } } diff --git a/packages/js/src/server/check-ins-manager/index.ts b/packages/js/src/server/check-ins-manager/index.ts index 1499d03a..5e1b8ec4 100644 --- a/packages/js/src/server/check-ins-manager/index.ts +++ b/packages/js/src/server/check-ins-manager/index.ts @@ -14,6 +14,7 @@ export class CheckInsManager { constructor(config: Partial, client?: CheckInsClient) { this.config = { debug: config.debug ?? false, + apiKey: config.apiKey ?? undefined, personalAuthToken: config.personalAuthToken ?? undefined, checkins: config.checkins ?? [], logger: config.logger ?? console, @@ -21,12 +22,18 @@ export class CheckInsManager { const transport = new ServerTransport() this.logger = Util.logger(this) this.client = client ?? new CheckInsClient({ + apiKey: config.apiKey, personalAuthToken: config.personalAuthToken, logger: this.logger }, transport) } public async sync(): Promise { + // check if api key is set + if (!this.config.apiKey || this.config.apiKey === '') { + throw new Error('apiKey is required') + } + // check if personal auth token is set if (!this.config.personalAuthToken || this.config.personalAuthToken === '') { throw new Error('personalAuthToken is required') @@ -51,12 +58,11 @@ export class CheckInsManager { return checkIn }); - // validate that we have unique check-in names - // throw error if there are check-ins with the same name and project id - const checkInNames = localCheckIns.map((checkIn) => `${checkIn.projectId}_${checkIn.name}`) - const uniqueCheckInNames = new Set(checkInNames) - if (checkInNames.length !== uniqueCheckInNames.size) { - throw new Error('check-in names must be unique per project') + // validate that we have unique check-in slugs + const checkInSlugs = localCheckIns.map((checkIn) => checkIn.slug) + const uniqueCheckInSlugs = new Set(checkInSlugs) + if (checkInSlugs.length !== uniqueCheckInSlugs.size) { + throw new Error('check-in slugs must be unique') } return localCheckIns @@ -64,19 +70,20 @@ export class CheckInsManager { private async createOrUpdate(localCheckIns: CheckIn[]) { const results = [] + const projectId = await this.client.getProjectId(this.config.apiKey) + const remoteCheckins = await this.client.listForProject(projectId) // for each check-in from the localCheckIns array, check if it exists in the API // if it does not exist, create it // if it exists, check if it needs to be updated for (const localCheckIn of localCheckIns) { - const projectCheckIns = await this.client.listForProject(localCheckIn.projectId) - const remoteCheckIn = projectCheckIns.find((checkIn) => { - return checkIn.name === localCheckIn.name + const remoteCheckIn = remoteCheckins.find((checkIn) => { + return checkIn.slug === localCheckIn.slug }) if (!remoteCheckIn) { - results.push(await this.client.create(localCheckIn)); + results.push(await this.client.create(projectId, localCheckIn)); } else if (!localCheckIn.isInSync(remoteCheckIn)) { localCheckIn.id = remoteCheckIn.id; - results.push(await this.client.update(localCheckIn)); + results.push(await this.client.update(projectId, localCheckIn)); } else { // no change - still need to add to results results.push(remoteCheckIn); @@ -87,22 +94,19 @@ export class CheckInsManager { } private async remove(localCheckIns: CheckIn[]) { - // get all project ids from local check-ins - // for each project id, get all check-ins from the API + const projectId = await this.client.getProjectId(this.config.apiKey) + const remoteCheckins = await this.client.listForProject(projectId) + + // get all check-ins from the API // if not found in local check-ins, remove it - const projectIds = Array.from(new Set(localCheckIns.map((checkIn) => checkIn.projectId))) - const remoteCheckInsPerProject = await Promise.all(projectIds.map((projectId) => { - return this.client.listForProject(projectId) - })) - const allRemoteCheckIns = remoteCheckInsPerProject.flat() - const checkInsToRemove = allRemoteCheckIns.filter((remoteCheckIn) => { + const checkInsToRemove = remoteCheckins.filter((remoteCheckIn) => { return !localCheckIns.find((localCheckIn) => { - return localCheckIn.name === remoteCheckIn.name + return localCheckIn.slug === remoteCheckIn.slug }) }) return Promise.all(checkInsToRemove.map(async (checkIn) => { - await this.client.remove(checkIn) + await this.client.remove(projectId, checkIn) checkIn.markAsDeleted() return checkIn diff --git a/packages/js/src/server/check-ins-manager/types.ts b/packages/js/src/server/check-ins-manager/types.ts index 6126a05a..c30530b3 100644 --- a/packages/js/src/server/check-ins-manager/types.ts +++ b/packages/js/src/server/check-ins-manager/types.ts @@ -9,12 +9,12 @@ export type CheckInDto = { /** * Checkin name. */ - name: string + name?: string /** * Checkin slug. */ - slug?: string + slug: string /** * Valid values are "simple" or "cron". @@ -45,24 +45,19 @@ export type CheckInDto = { * Valid timezone values are listed here {@link https://docs.honeybadger.io/api/check-ins/timezones here}. */ cronTimezone?: string - - /** - * The project ID that this checkin belongs to. - */ - projectId: string - } export type CheckInsConfig = { debug?: boolean logger?: Types.Logger + apiKey: string personalAuthToken: string checkins: CheckInDto[] } export type CheckInPayload = { - name: string - slug?: string + name?: string + slug: string schedule_type: 'simple' | 'cron' report_period?: string grace_period?: string diff --git a/packages/js/test/unit/server.test.ts b/packages/js/test/unit/server.test.ts index ec16371b..657e31f1 100644 --- a/packages/js/test/unit/server.test.ts +++ b/packages/js/test/unit/server.test.ts @@ -387,8 +387,7 @@ describe('server client', function () { it('sends a check-in report using a check-in name', async function () { const checkInId = '123' const checkInConfig: CheckInDto = { - name: 'a simple check-in', - projectId: '1111', + slug: 'a-simple-check-in', scheduleType: 'simple', reportPeriod: '1 day', } @@ -400,13 +399,24 @@ describe('server client', function () { }) const localCheckIn = new CheckIn(checkInConfig) + const getProjectIdRequest = nock('https://app.honeybadger.io') + .get('/v2/project_keys/testing') + .once() + .reply(200, { + id: 'testing', + project: { + id: '11111', + name: 'Test', + } + }) + const listProjectCheckInsRequest = nock('https://app.honeybadger.io') - .get(`/v2/projects/${localCheckIn.projectId}/check_ins`) + .get('/v2/projects/11111/check_ins') .once() .reply(200, { results: [ { - ...new CheckIn(checkInConfig).asRequestPayload(), + ...localCheckIn.asRequestPayload(), id: checkInId }, ] as CheckInResponsePayload[] @@ -416,7 +426,8 @@ describe('server client', function () { .get(`/v1/check_in/${checkInId}`) .reply(201) - await client.checkIn(checkInConfig.name) + await client.checkIn(checkInConfig.slug) + expect(getProjectIdRequest.isDone()).toBe(true) expect(listProjectCheckInsRequest.isDone()).toBe(true) expect(checkinRequest.isDone()).toBe(true) }) @@ -424,8 +435,7 @@ describe('server client', function () { it('sends a check-in report using a check-in name without calling the HB API', async function () { const checkInId = '123' const checkInConfig: CheckInDto = { - name: 'a simple check-in', - projectId: '1111', + slug: 'a-simple-check-in', scheduleType: 'simple', reportPeriod: '1 day', } @@ -437,13 +447,24 @@ describe('server client', function () { }) const localCheckIn = new CheckIn(checkInConfig) + const getProjectIdRequest = nock('https://app.honeybadger.io') + .get('/v2/project_keys/testing') + .once() + .reply(200, { + id: 'testing', + project: { + id: '11111', + name: 'Test', + } + }) + const listProjectCheckInsRequest = nock('https://app.honeybadger.io') - .get(`/v2/projects/${localCheckIn.projectId}/check_ins`) + .get('/v2/projects/11111/check_ins') .once() .reply(200, { results: [ { - ...new CheckIn(checkInConfig).asRequestPayload(), + ...localCheckIn.asRequestPayload(), id: checkInId }, ] as CheckInResponsePayload[] @@ -454,8 +475,9 @@ describe('server client', function () { .twice() .reply(201) - await client.checkIn(checkInConfig.name) - await client.checkIn(checkInConfig.name) + await client.checkIn(checkInConfig.slug) + await client.checkIn(checkInConfig.slug) + expect(getProjectIdRequest.isDone()).toBe(true) expect(listProjectCheckInsRequest.isDone()).toBe(true) expect(checkInRequest.isDone()).toBe(true) }) diff --git a/packages/js/test/unit/server/check-ins-sync.test.ts b/packages/js/test/unit/server/check-ins-sync.test.ts index 77e431f8..7208ee69 100644 --- a/packages/js/test/unit/server/check-ins-sync.test.ts +++ b/packages/js/test/unit/server/check-ins-sync.test.ts @@ -12,32 +12,27 @@ describe('check-ins-sync', () => { await expect(syncCheckIns()).rejects.toThrow('Could not find a Honeybadger configuration file.') }) - it('should throw an error if personal auth token is not set', async () => { + it('should throw an error if api key is not set', async () => { jest.doMock('../../../honeybadger.config.js', () => ({}), { virtual: true }) - await expect(syncCheckIns()).rejects.toThrow('personalAuthToken is required') + await expect(syncCheckIns()).rejects.toThrow('apiKey is required') }) - it('should not sync if check-ins array is empty', async () => { + it('should throw an error if personal auth token is not set', async () => { jest.doMock('../../../honeybadger.config.js', () => ({ - personalAuthToken: '123' + apiKey: 'hbp_123', }), { virtual: true }) - const consoleLogSpy = jest.spyOn(console, 'log') - const consoleErrorSpy = jest.spyOn(console, 'error') - - await syncCheckIns() - expect(consoleErrorSpy).not.toHaveBeenCalled() - expect(consoleLogSpy).toHaveBeenCalledWith('No check-ins found to synchronize with Honeybadger.') + await expect(syncCheckIns()).rejects.toThrow('personalAuthToken is required') }) it('should sync checkIns', async () => { const checkInsConfig: Partial = { + apiKey: 'hbp_123', personalAuthToken: '123', checkins: [ { - projectId: '11111', - name: 'a check-in', + slug: 'a-check-in', scheduleType: 'simple', reportPeriod: '1 week', gracePeriod: '5 minutes' @@ -49,8 +44,19 @@ describe('check-ins-sync', () => { const consoleLogSpy = jest.spyOn(console, 'log') const consoleErrorSpy = jest.spyOn(console, 'error') + const getProjectIdRequest = nock('https://app.honeybadger.io') + .get(`/v2/project_keys/${checkInsConfig.apiKey}`) + .once() + .reply(200, { + id: checkInsConfig.apiKey, + project: { + id: '11111', + name: 'Test', + } + }) + const listProjectCheckInsRequest = nock('https://app.honeybadger.io') - .get(`/v2/projects/${checkInsConfig.checkins[0].projectId}/check_ins`) + .get('/v2/projects/11111/check_ins') .once() .reply(200, { results: [ @@ -62,6 +68,7 @@ describe('check-ins-sync', () => { }) await syncCheckIns() + expect(getProjectIdRequest.isDone()).toBe(true) expect(listProjectCheckInsRequest.isDone()).toBe(true) expect(consoleErrorSpy).not.toHaveBeenCalled() expect(consoleLogSpy).toHaveBeenCalledWith('Check-ins were synchronized with Honeybadger.') diff --git a/packages/js/test/unit/server/checkins-manager/checkin.test.ts b/packages/js/test/unit/server/checkins-manager/checkin.test.ts index 87fe3729..cac5cb61 100644 --- a/packages/js/test/unit/server/checkins-manager/checkin.test.ts +++ b/packages/js/test/unit/server/checkins-manager/checkin.test.ts @@ -2,60 +2,51 @@ import { CheckIn } from '../../../../src/server/check-ins-manager/check-in'; describe('CheckIn', function () { it('should be able to create a check-in', function () { - const checkIn = new CheckIn({ name: 'a check-in', projectId: '123', scheduleType: 'simple' }) + const checkIn = new CheckIn({ slug: 'a-check-in', scheduleType: 'simple' }) expect(checkIn).toBeInstanceOf(CheckIn) }) it('should be able to mark a checkin as deleted', function () { - const checkIn = new CheckIn({ name: 'a check-in', projectId: '123', scheduleType: 'simple' }) + const checkIn = new CheckIn({ slug: 'a-check-in', scheduleType: 'simple' }) checkIn.markAsDeleted() expect(checkIn.isDeleted()).toBeTruthy() }) describe('validate', function () { - it('should throw an error if the check-in is missing a projectId', function () { + it('should throw an error if the check-in is missing a slug', function () { // @ts-expect-error - const checkIn = new CheckIn({ name: 'a check-in' }) - expect(() => checkIn.validate()).toThrowError('projectId is required for each check-in') - }) - - it('should throw an error if the check-in is missing a name', function () { - // @ts-expect-error - const checkIn = new CheckIn({ projectId: '11111' }) - expect(() => checkIn.validate()).toThrowError('name is required for each check-in') + const checkIn = new CheckIn({}) + expect(() => checkIn.validate()).toThrowError('slug is required for each check-in') }) it('should throw an error if the check-in is missing a scheduleType', function () { // @ts-expect-error - const checkIn = new CheckIn({ projectId: '11111', name: 'a check-in' }) + const checkIn = new CheckIn({ slug: 'a-check-in' }) expect(() => checkIn.validate()).toThrowError('scheduleType is required for each check-in') }) it('should throw an error if the check-in has an invalid scheduleType', function () { const checkIn = new CheckIn({ - projectId: '11111', - name: 'a check-in', + slug: 'a-check-in', scheduleType: 'invalid' as never }) - expect(() => checkIn.validate()).toThrowError('a check-in [scheduleType] must be "simple" or "cron"') + expect(() => checkIn.validate()).toThrowError('a-check-in [scheduleType] must be "simple" or "cron"') }) it('should throw an error if the check-in is missing a reportPeriod', function () { const checkIn = new CheckIn({ - projectId: '11111', - name: 'a check-in', + slug: 'a-check-in', scheduleType: 'simple' }) - expect(() => checkIn.validate()).toThrowError('a check-in [reportPeriod] is required for simple check-ins') + expect(() => checkIn.validate()).toThrowError('a-check-in [reportPeriod] is required for simple check-ins') }) it('should throw an error if the check-in is missing a cronSchedule', function () { const checkIn = new CheckIn({ - projectId: '11111', - name: 'a check-in', + slug: 'a-check-in', scheduleType: 'cron' }) - expect(() => checkIn.validate()).toThrowError('a check-in [cronSchedule] is required for cron check-ins') + expect(() => checkIn.validate()).toThrowError('a-check-in [cronSchedule] is required for cron check-ins') }) }) }) diff --git a/packages/js/test/unit/server/checkins-manager/client.test.ts b/packages/js/test/unit/server/checkins-manager/client.test.ts index 558a7cca..c14fbc76 100644 --- a/packages/js/test/unit/server/checkins-manager/client.test.ts +++ b/packages/js/test/unit/server/checkins-manager/client.test.ts @@ -8,11 +8,32 @@ describe('CheckinsClient', () => { it('should create a client', () => { const client = new CheckInsClient({ logger: nullLogger(), + apiKey: 'hbp_123', personalAuthToken: '123', }, new ServerTransport()) expect(client).toBeInstanceOf(CheckInsClient) }) + it('should get project id from an api key', async () => { + const request = nock('https://app.honeybadger.io') + .get('/v2/project_keys/hbp_123') + .reply(200, { + project: { + id: '11111', + name: 'a project', + } + }) + + const client = new CheckInsClient({ + logger: nullLogger(), + apiKey: 'hbp_123', + personalAuthToken: '123', + }, new ServerTransport()) + const projectId = await client.getProjectId('hbp_123') + expect(request.isDone()).toBe(true) + expect(projectId).toEqual('11111') + }) + it('should list check-ins for a project', async () => { const projectId = '11111' const request = nock('https://app.honeybadger.io') @@ -21,7 +42,7 @@ describe('CheckinsClient', () => { results: [ { id: 'uuid', - name: 'a check-in', + slug: 'a-check-in', schedule_type: 'simple', report_period: '1 week', } @@ -29,13 +50,14 @@ describe('CheckinsClient', () => { }) const client = new CheckInsClient({ logger: nullLogger(), + apiKey: 'hbp_123', personalAuthToken: '123', }, new ServerTransport()) const checkIns = await client.listForProject(projectId) expect(request.isDone()).toBe(true) expect(checkIns).toHaveLength(1) expect(checkIns[0].id).toEqual('uuid') - expect(checkIns[0].name).toEqual('a check-in') + expect(checkIns[0].slug).toEqual('a-check-in') expect(checkIns[0].scheduleType).toEqual('simple') expect(checkIns[0].reportPeriod).toEqual('1 week') }) @@ -49,7 +71,7 @@ describe('CheckinsClient', () => { results: [ { id: 'uuid', - name: 'a check-in', + slug: 'a-check-in', schedule_type: 'simple', report_period: '1 week', } @@ -57,6 +79,7 @@ describe('CheckinsClient', () => { }) const client = new CheckInsClient({ logger: nullLogger(), + apiKey: 'hbp_123', personalAuthToken: '123', }, new ServerTransport()) const checkIns = await client.listForProject(projectId) @@ -74,18 +97,19 @@ describe('CheckinsClient', () => { .once() .reply(200, { id: 'uuid', - name: 'a check-in', + slug: 'a-check-in', schedule_type: 'simple', report_period: '1 week', }) const client = new CheckInsClient({ logger: nullLogger(), + apiKey: 'hbp_123', personalAuthToken: '123', }, new ServerTransport()) const checkIn = await client.get(projectId, checkInId) expect(request.isDone()).toBe(true) expect(checkIn.id).toEqual('uuid') - expect(checkIn.name).toEqual('a check-in') + expect(checkIn.slug).toEqual('a-check-in') expect(checkIn.scheduleType).toEqual('simple') expect(checkIn.reportPeriod).toEqual('1 week') }) @@ -95,8 +119,7 @@ describe('CheckinsClient', () => { const checkInId = '22222' const checkInToBeSaved = new CheckIn({ - projectId, - name: 'a check-in', + slug: 'a-check-in', scheduleType: 'simple', reportPeriod: '1 week', }) @@ -111,12 +134,13 @@ describe('CheckinsClient', () => { }) const client = new CheckInsClient({ logger: nullLogger(), + apiKey: 'hbp_123', personalAuthToken: '123', }, new ServerTransport()) - const checkIn = await client.create(checkInToBeSaved) + const checkIn = await client.create(projectId, checkInToBeSaved) expect(request.isDone()).toBe(true) expect(checkIn.id).toEqual(checkInId) - expect(checkIn.name).toEqual('a check-in') + expect(checkIn.slug).toEqual('a-check-in') expect(checkIn.scheduleType).toEqual('simple') expect(checkIn.reportPeriod).toEqual('1 week') }) @@ -126,9 +150,8 @@ describe('CheckinsClient', () => { const checkInId = '22222' const checkInToBeUpdated = new CheckIn({ - projectId, id: checkInId, - name: 'a check-in', + slug: 'a-check-in', scheduleType: 'simple', reportPeriod: '1 week', }) @@ -143,12 +166,13 @@ describe('CheckinsClient', () => { }) const client = new CheckInsClient({ logger: nullLogger(), + apiKey: 'hbp_123', personalAuthToken: '123', }, new ServerTransport()) - const checkIn = await client.update(checkInToBeUpdated) + const checkIn = await client.update(projectId, checkInToBeUpdated) expect(request.isDone()).toBe(true) expect(checkIn.id).toEqual(checkInId) - expect(checkIn.name).toEqual('a check-in') + expect(checkIn.slug).toEqual('a-check-in') expect(checkIn.scheduleType).toEqual('simple') expect(checkIn.reportPeriod).toEqual('1 week') }) @@ -158,9 +182,8 @@ describe('CheckinsClient', () => { const checkInId = '22222' const checkInToBeRemoved = new CheckIn({ - projectId, id: checkInId, - name: 'a check-in', + slug: 'a-check-in', scheduleType: 'simple', reportPeriod: '1 week', }) @@ -172,10 +195,11 @@ describe('CheckinsClient', () => { const client = new CheckInsClient({ logger: nullLogger(), + apiKey: 'hbp_123', personalAuthToken: '123', }, new ServerTransport()) - await client.remove(checkInToBeRemoved) + await client.remove(projectId, checkInToBeRemoved) expect(request.isDone()).toBe(true) }) }) diff --git a/packages/js/test/unit/server/checkins-manager/manager.test.ts b/packages/js/test/unit/server/checkins-manager/manager.test.ts index ef42ef05..faf72a7a 100644 --- a/packages/js/test/unit/server/checkins-manager/manager.test.ts +++ b/packages/js/test/unit/server/checkins-manager/manager.test.ts @@ -8,6 +8,7 @@ describe('CheckinsManager', () => { it('should create a check-ins manager', () => { const config: CheckInsConfig = { logger: nullLogger(), + apiKey: 'hbp_abc', personalAuthToken: 'abc', checkins: [] } @@ -15,109 +16,67 @@ describe('CheckinsManager', () => { expect(manager).toBeInstanceOf(CheckInsManager) }) - it('should throw if personal auth token is not set', () => { + it('should throw if api key is not set', () => { const config: CheckInsConfig = { logger: nullLogger(), + apiKey: '', personalAuthToken: '', checkins: [] } const manager = new CheckInsManager(config) - expect(manager.sync()).rejects.toThrow('personalAuthToken is required') + expect(manager.sync()).rejects.toThrow('apiKey is required') }) - it('should throw if a check-in configuration is invalid', () => { + it('should throw if personal auth token is not set', () => { const config: CheckInsConfig = { logger: nullLogger(), - personalAuthToken: 'abc', - checkins: [ - { - projectId: '11111', - name: 'a check-in', - reportPeriod: '1 week', - gracePeriod: '5 minutes' - }, - ] as CheckInDto[] + apiKey: 'hbp_123', + personalAuthToken: '', + checkins: [] } const manager = new CheckInsManager(config) - expect(manager.sync()).rejects.toThrow('scheduleType is required for each check-in') + expect(manager.sync()).rejects.toThrow('personalAuthToken is required') }) - it('should throw if check-in names are not unique per project', () => { + it('should throw if a check-in configuration is invalid', () => { const config: CheckInsConfig = { logger: nullLogger(), + apiKey: 'hbp_abc', personalAuthToken: 'abc', checkins: [ { - projectId: '11111', - name: 'a check-in', - scheduleType: 'simple', + slug: 'a-check-in', reportPeriod: '1 week', gracePeriod: '5 minutes' }, - { - projectId: '11111', - name: 'a check-in', - scheduleType: 'simple', - reportPeriod: '2 weeks', - gracePeriod: '5 minutes' - }, ] as CheckInDto[] } const manager = new CheckInsManager(config) - expect(manager.sync()).rejects.toThrow('check-in names must be unique per project') + expect(manager.sync()).rejects.toThrow('scheduleType is required for each check-in') }) - it('should not throw if check-in names are not unique but have different project id', async () => { + it('should throw if check-in slugs are not unique', () => { const config: CheckInsConfig = { logger: nullLogger(), + apiKey: 'hbp_abc', personalAuthToken: 'abc', checkins: [ { - projectId: '11111', - name: 'a check-in', + slug: 'a-check-in', scheduleType: 'simple', reportPeriod: '1 week', gracePeriod: '5 minutes' }, { - projectId: '22222', - name: 'a check-in', + slug: 'a-check-in', scheduleType: 'simple', reportPeriod: '2 weeks', gracePeriod: '5 minutes' }, ] as CheckInDto[] } - - const listProjectCheckInsRequest1 = nock('https://app.honeybadger.io') - .get('/v2/projects/11111/check_ins') - .once() - .reply(200, { - results: [ - { - id: 'abc', - ...(new CheckIn(config.checkins[0]).asRequestPayload()) - } - ] - }) - - const listProjectCheckInsRequest2 = nock('https://app.honeybadger.io') - .get('/v2/projects/22222/check_ins') - .once() - .reply(200, { - results: [ - { - id: 'def', - ...(new CheckIn(config.checkins[1]).asRequestPayload()) - } - ] - }) - const manager = new CheckInsManager(config) - const synchronizedCheckIns = await manager.sync() - expect(listProjectCheckInsRequest1.isDone()).toBe(true) - expect(listProjectCheckInsRequest2.isDone()).toBe(true) - expect(synchronizedCheckIns).toHaveLength(2) + expect(manager.sync()).rejects.toThrow('check-in slugs must be unique') }) it('should create check-ins from config', async () => { @@ -126,18 +85,17 @@ describe('CheckinsManager', () => { const cronCheckInId = '33333' const config: CheckInsConfig = { logger: nullLogger(), + apiKey: 'hbp_abc', personalAuthToken: 'abc', checkins: [ { - projectId, - name: 'a check-in', + slug: 'a-check-in', scheduleType: 'simple', reportPeriod: '1 week', gracePeriod: '5 minutes' }, { - projectId, - name: 'a cron check-in', + slug: 'a-cron-check-in', scheduleType: 'cron', cronSchedule: '* * * * 5', gracePeriod: '25 minutes' @@ -145,6 +103,17 @@ describe('CheckinsManager', () => { ] } + const getProjectIdRequest = nock('https://app.honeybadger.io') + .get(`/v2/project_keys/${config.apiKey}`) + .once() + .reply(200, { + id: config.apiKey, + project: { + id: projectId, + name: 'Test', + } + }) + const listProjectCheckInsRequest = nock('https://app.honeybadger.io') .get(`/v2/projects/${projectId}/check_ins`) .once() @@ -176,6 +145,7 @@ describe('CheckinsManager', () => { const manager = new CheckInsManager(config) const synchronizedCheckIns = await manager.sync() + expect(getProjectIdRequest.isDone()).toBe(true) expect(listProjectCheckInsRequest.isDone()).toBe(true) expect(createSimpleCheckInRequest.isDone()).toBe(true) expect(createCronCheckInRequest.isDone()).toBe(true) @@ -192,18 +162,17 @@ describe('CheckinsManager', () => { const cronCheckInId = '33333' const config: CheckInsConfig = { logger: nullLogger(), + apiKey: 'hbp_abc', personalAuthToken: 'abc', checkins: [ { - projectId, - name: 'a check-in', + slug: 'a-check-in', scheduleType: 'simple', reportPeriod: '1 week', gracePeriod: '15 minutes' // the value to update }, { - projectId, - name: 'a cron check-in', + slug: 'a-cron-check-in', scheduleType: 'cron', cronSchedule: '* * * 1 5', // the value to update gracePeriod: '25 minutes' @@ -211,6 +180,17 @@ describe('CheckinsManager', () => { ] } + const getProjectIdRequest = nock('https://app.honeybadger.io') + .get(`/v2/project_keys/${config.apiKey}`) + .once() + .reply(200, { + id: config.apiKey, + project: { + id: projectId, + name: 'Test', + } + }) + const listProjectCheckInsRequest = nock('https://app.honeybadger.io') .get(`/v2/projects/${projectId}/check_ins`) .once() @@ -218,14 +198,14 @@ describe('CheckinsManager', () => { results: [ { id: simpleCheckInId, - name: 'a check-in', + slug: 'a-check-in', schedule_type: 'simple', report_period: '1 week', grace_period: '5 minutes' }, { id: cronCheckInId, - name: 'a cron check-in', + slug: 'a-cron-check-in', scheduleType: 'cron', cronSchedule: '* * * * 5', gracePeriod: '25 minutes' @@ -251,6 +231,7 @@ describe('CheckinsManager', () => { const manager = new CheckInsManager(config) const synchronizedCheckIns = await manager.sync() + expect(getProjectIdRequest.isDone()).toBe(true) expect(listProjectCheckInsRequest.isDone()).toBe(true) expect(updateSimpleCheckInRequest.isDone()).toBe(true) expect(updateCronCheckInRequest.isDone()).toBe(true) @@ -267,22 +248,33 @@ describe('CheckinsManager', () => { }) }) - it('should update check-ins from config to unset optional values', async () => { + it('should not update check-ins from config if no values changed', async () => { const projectId = '11111' const simpleCheckInId = '22222' const config: CheckInsConfig = { logger: nullLogger(), + apiKey: 'hbp_abc', personalAuthToken: 'abc', checkins: [ { - projectId, - name: 'a check-in', + slug: 'a-check-in', scheduleType: 'simple', reportPeriod: '1 week', }, ] } + const getProjectIdRequest = nock('https://app.honeybadger.io') + .get(`/v2/project_keys/${config.apiKey}`) + .once() + .reply(200, { + id: config.apiKey, + project: { + id: projectId, + name: 'Test', + } + }) + const listProjectCheckInsRequest = nock('https://app.honeybadger.io') .get(`/v2/projects/${projectId}/check_ins`) .once() @@ -290,7 +282,7 @@ describe('CheckinsManager', () => { results: [ { id: simpleCheckInId, - name: 'a check-in', + name: 'a random generated value that should not cause an update', slug: 'a-check-in', schedule_type: 'simple', report_period: '1 week', @@ -298,18 +290,10 @@ describe('CheckinsManager', () => { ] as CheckInResponsePayload[] }) - const simpleCheckInPayload = new CheckIn(config.checkins[0]).asRequestPayload() - const updateSimpleCheckInRequest = nock('https://app.honeybadger.io') - .put(`/v2/projects/${projectId}/check_ins/${simpleCheckInId}`, { - check_in: simpleCheckInPayload - }) - .once() - .reply(204) - const manager = new CheckInsManager(config) const synchronizedCheckIns = await manager.sync() + expect(getProjectIdRequest.isDone()).toBe(true) expect(listProjectCheckInsRequest.isDone()).toBe(true) - expect(updateSimpleCheckInRequest.isDone()).toBe(true) expect(synchronizedCheckIns).toHaveLength(1) expect(synchronizedCheckIns[0]).toMatchObject({ ...config.checkins[0], @@ -323,11 +307,11 @@ describe('CheckinsManager', () => { const checkInIdToRemove = '33333' const config: CheckInsConfig = { logger: nullLogger(), + apiKey: 'hbp_abc', personalAuthToken: 'abc', checkins: [ { - projectId, - name: 'a check-in', + slug: 'a-check-in', scheduleType: 'simple', reportPeriod: '1 week', gracePeriod: '5 minutes' @@ -335,6 +319,17 @@ describe('CheckinsManager', () => { ] } + const getProjectIdRequest = nock('https://app.honeybadger.io') + .get(`/v2/project_keys/${config.apiKey}`) + .once() + .reply(200, { + id: config.apiKey, + project: { + id: projectId, + name: 'Test', + } + }) + const listProjectCheckInsRequest = nock('https://app.honeybadger.io') .get(`/v2/projects/${projectId}/check_ins`) .once() @@ -342,14 +337,14 @@ describe('CheckinsManager', () => { results: [ { id: simpleCheckInId, - name: 'a check-in', + slug: 'a-check-in', schedule_type: 'simple', report_period: '1 week', grace_period: '5 minutes' }, { id: checkInIdToRemove, - name: 'a cron check-in', + slug: 'a-cron-check-in', scheduleType: 'cron', cronSchedule: '* * * * 5', gracePeriod: '25 minutes' @@ -364,6 +359,7 @@ describe('CheckinsManager', () => { const manager = new CheckInsManager(config) const synchronizedCheckIns = await manager.sync() + expect(getProjectIdRequest.isDone()).toBe(true) expect(listProjectCheckInsRequest.isDone()).toBe(true) expect(removeCheckInRequest.isDone()).toBe(true) expect(synchronizedCheckIns).toHaveLength(2)