From c4ee8bb0a28800e0800109b58a4cc9df4414994b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Patrick=20M=C3=BCller?= Date: Sun, 30 Aug 2026 16:12:30 +0200 Subject: [PATCH] Log .ics build failures; add salesforce.client unit tests Review follow-up: - tickets.confirmation-email.ts: a failed .ics generation was swallowed silently; log a warning (the email still goes out without the attachment) - test/common/salesforce.client.test.ts: direct coverage for the shared client's token cache and retry-once-on-401 (previously exercised only indirectly through the newsletter sync test) Co-Authored-By: Claude Sonnet 5 --- .../tickets/tickets.confirmation-email.ts | 4 +- test/common/salesforce.client.test.ts | 124 ++++++++++++++++++ test/tickets/confirmation-email.test.ts | 5 +- 3 files changed, 131 insertions(+), 2 deletions(-) create mode 100644 test/common/salesforce.client.test.ts diff --git a/src/models/tickets/tickets.confirmation-email.ts b/src/models/tickets/tickets.confirmation-email.ts index a2984a4..8a8b488 100644 --- a/src/models/tickets/tickets.confirmation-email.ts +++ b/src/models/tickets/tickets.confirmation-email.ts @@ -49,7 +49,9 @@ export const sendRedemptionConfirmation = async (recipient: ConfirmationRecipien try { const ics = await IcalService.convertToIcal([event]); icsAttachment = [{filename: 'konzert.ics', content: ics, contentType: 'text/calendar'}]; - } catch { + } catch (e: any) { + // Non-fatal: the confirmation still goes out, just without the calendar file. + logger.warn('Confirmation email for event ' + recipient.eventId + ' sent without .ics attachment: ' + e?.message); icsAttachment = undefined; } diff --git a/test/common/salesforce.client.test.ts b/test/common/salesforce.client.test.ts new file mode 100644 index 0000000..8f73d2c --- /dev/null +++ b/test/common/salesforce.client.test.ts @@ -0,0 +1,124 @@ +// salesforce.client caches the OAuth token at module scope, so every test +// resets the module registry for a clean cache and re-requires axios + the +// module under test after the reset (same approach as +// test/feedback/salesforce.service.test.ts). + +export {}; // isolate module scope from other script-style test files + +jest.mock('axios'); + +const freshImports = () => { + const axios = require('axios'); + const {salesforceApexRestPost, salesforceEnabled} = require('../../src/common/salesforce.client'); + return {axios, salesforceApexRestPost, salesforceEnabled}; +}; + +const ORIGINAL_ENV = {...process.env}; +const ENABLED_ENV = { + ...ORIGINAL_ENV, + SALESFORCE_ENABLED: 'true', + SALESFORCE_API_URL: 'https://example.my.salesforce.com', + SALESFORCE_CLIENT_ID: 'client-id', + SALESFORCE_CLIENT_SECRET: 'client-secret' +}; + +beforeEach(() => { + jest.resetModules(); + process.env = {...ENABLED_ENV}; +}); + +afterAll(() => { + process.env = {...ORIGINAL_ENV}; +}); + +describe('salesforceEnabled', () => { + it('is true only when SALESFORCE_ENABLED === "true"', () => { + process.env.SALESFORCE_ENABLED = 'true'; + expect(freshImports().salesforceEnabled()).toBe(true); + + jest.resetModules(); + process.env.SALESFORCE_ENABLED = 'false'; + expect(freshImports().salesforceEnabled()).toBe(false); + }); +}); + +describe('salesforceApexRestPost', () => { + it('fetches a token, posts to the given Apex REST path, and returns the response body', async () => { + const {axios, salesforceApexRestPost} = freshImports(); + axios.post.mockImplementation((url: string) => { + if (url.endsWith('/services/oauth2/token')) return Promise.resolve({data: {access_token: 'tok-1'}}); + return Promise.resolve({data: {ok: true}}); + }); + + const result = await salesforceApexRestPost('/services/apexrest/email/send', {to: 'x@example.com'}); + + expect(result).toEqual({ok: true}); + expect(axios.post).toHaveBeenCalledWith( + 'https://example.my.salesforce.com/services/oauth2/token', + expect.any(String), + expect.objectContaining({headers: {'Content-Type': 'application/x-www-form-urlencoded'}}) + ); + expect(axios.post).toHaveBeenCalledWith( + 'https://example.my.salesforce.com/services/apexrest/email/send', + {to: 'x@example.com'}, + expect.objectContaining({headers: {Authorization: 'Bearer tok-1'}}) + ); + }); + + it('reuses the cached token across calls instead of fetching twice', async () => { + const {axios, salesforceApexRestPost} = freshImports(); + axios.post.mockImplementation((url: string) => { + if (url.endsWith('/services/oauth2/token')) return Promise.resolve({data: {access_token: 'tok-1'}}); + return Promise.resolve({data: {}}); + }); + + await salesforceApexRestPost('/services/apexrest/email/send', {}); + await salesforceApexRestPost('/services/apexrest/email/send', {}); + + const tokenCalls = axios.post.mock.calls.filter(([url]: [string]) => url.endsWith('/services/oauth2/token')); + expect(tokenCalls).toHaveLength(1); + }); + + it('retries once with a fresh token on a 401, then succeeds', async () => { + const {axios, salesforceApexRestPost} = freshImports(); + let tokenFetches = 0; + axios.post.mockImplementation((url: string) => { + if (url.endsWith('/services/oauth2/token')) { + tokenFetches += 1; + return Promise.resolve({data: {access_token: `tok-${tokenFetches}`}}); + } + if (tokenFetches === 1) { + const err: any = new Error('Unauthorized'); + err.response = {status: 401}; + return Promise.reject(err); + } + return Promise.resolve({data: {ok: true}}); + }); + + const result = await salesforceApexRestPost('/services/apexrest/email/send', {}); + + expect(result).toEqual({ok: true}); + expect(tokenFetches).toBe(2); + }); + + it('does not retry on a non-401 error and rethrows it', async () => { + const {axios, salesforceApexRestPost} = freshImports(); + axios.post.mockImplementation((url: string) => { + if (url.endsWith('/services/oauth2/token')) return Promise.resolve({data: {access_token: 'tok-1'}}); + const err: any = new Error('Server error'); + err.response = {status: 500}; + return Promise.reject(err); + }); + + await expect(salesforceApexRestPost('/services/apexrest/email/send', {})).rejects.toThrow('Server error'); + const endpointCalls = axios.post.mock.calls.filter(([url]: [string]) => url.endsWith('/email/send')); + expect(endpointCalls).toHaveLength(1); + }); + + it('throws a clear error when client credentials are not configured', async () => { + process.env.SALESFORCE_CLIENT_ID = ''; + const {salesforceApexRestPost} = freshImports(); + + await expect(salesforceApexRestPost('/services/apexrest/email/send', {})).rejects.toThrow('SALESFORCE_CLIENT_ID'); + }); +}); diff --git a/test/tickets/confirmation-email.test.ts b/test/tickets/confirmation-email.test.ts index d54250e..4e8a13e 100644 --- a/test/tickets/confirmation-email.test.ts +++ b/test/tickets/confirmation-email.test.ts @@ -21,12 +21,14 @@ jest.mock('../../src/middleware/logger', () => ({ import * as EventsService from '../../src/models/calendar/events/events.service'; import * as IcalService from '../../src/models/calendar/events/icalgenerator.service'; import {MailService} from '../../src/common/common.mail'; +import logger from '../../src/middleware/logger'; import {NachklangTicketsDB} from '../../src/models/tickets/Tickets.db'; import {recordConfirmationEmailResult, sendRedemptionConfirmation} from '../../src/models/tickets/tickets.confirmation-email'; const mockGetEvent = EventsService.getEventById as jest.Mock; const mockToIcal = IcalService.convertToIcal as jest.Mock; const mockSendMail = MailService.sendMail as jest.Mock; +const mockLogger = logger as unknown as {info: jest.Mock; warn: jest.Mock; error: jest.Mock}; const mockGetConnection = NachklangTicketsDB.getConnection as jest.Mock; const EVENT = { @@ -68,13 +70,14 @@ describe('sendRedemptionConfirmation', () => { ]); }); - it('still sends (without an attachment) when the .ics build fails', async () => { + it('still sends (without an attachment) and warns when the .ics build fails', async () => { mockToIcal.mockRejectedValue(new Error('ical boom')); await sendRedemptionConfirmation(RECIPIENT); const options = mockSendMail.mock.calls[0][3]; expect(options.attachments).toBeUndefined(); + expect(mockLogger.warn).toHaveBeenCalledWith(expect.stringContaining('without .ics attachment')); }); it('returns false and does not send when the event no longer exists', async () => {