refactor: move v1 API + CRM event-type un-hardcoding to a follow-up PR
Keeps #801 scoped to the setup-wizard event-types feature and its load-bearing guards. The v1 validator/discovery endpoint and the contract-conversion default fix ship separately so the public-API behavior change gets its own review weight.
This commit is contained in:
@@ -11,8 +11,6 @@
|
|||||||
* - reminder-template self-heal does NOT resurrect templates for slugs
|
* - reminder-template self-heal does NOT resurrect templates for slugs
|
||||||
* that no longer exist in the catalog
|
* that no longer exist in the catalog
|
||||||
* - after markSetupWizardCompleted() → system deletion is refused again
|
* - after markSetupWizardCompleted() → system deletion is refused again
|
||||||
* - resolveDefaultEventType never returns a hardcoded slug that the
|
|
||||||
* admin removed (contract→event conversion default, #800)
|
|
||||||
*/
|
*/
|
||||||
|
|
||||||
const { bootCrmDb } = require('./helpers/crmDb');
|
const { bootCrmDb } = require('./helpers/crmDb');
|
||||||
@@ -109,21 +107,6 @@ describe('event type deletion during the setup window (#800)', () => {
|
|||||||
expect(result.success).toBe(true);
|
expect(result.success).toBe(true);
|
||||||
});
|
});
|
||||||
|
|
||||||
it('resolveDefaultEventType follows the catalog instead of hardcoding a slug', async () => {
|
|
||||||
// 'other' is active → preferred catch-all.
|
|
||||||
expect(await eventTypeService.resolveDefaultEventType()).toBe('other');
|
|
||||||
|
|
||||||
// Deactivate 'other' → falls over to the first active type by display order.
|
|
||||||
const other = await db('event_types').where({ slug_prefix: 'other' }).first();
|
|
||||||
await db('event_types').where({ id: other.id }).update({ is_active: 0 });
|
|
||||||
const resolved = await eventTypeService.resolveDefaultEventType();
|
|
||||||
expect(resolved).not.toBe('other');
|
|
||||||
const resolvedRow = await db('event_types').where({ slug_prefix: resolved }).first();
|
|
||||||
expect(resolvedRow).toBeTruthy();
|
|
||||||
|
|
||||||
await db('event_types').where({ id: other.id }).update({ is_active: 1 });
|
|
||||||
});
|
|
||||||
|
|
||||||
it('fails closed when the completion marker row is missing', async () => {
|
it('fails closed when the completion marker row is missing', async () => {
|
||||||
// A portable-backup restore can replace app_settings with a set that
|
// A portable-backup restore can replace app_settings with a set that
|
||||||
// predates migration 161 (which will not rerun) — absence must mean
|
// predates migration 161 (which will not rerun) — absence must mean
|
||||||
|
|||||||
@@ -80,16 +80,7 @@ jest.mock('../../../services/webhookService', () => ({
|
|||||||
buildEventSubject: jest.fn().mockReturnValue({}),
|
buildEventSubject: jest.fn().mockReturnValue({}),
|
||||||
}));
|
}));
|
||||||
|
|
||||||
// event_type is validated against the live event_types catalog (#800) —
|
|
||||||
// that lookup would consume the first queued db() chain and shift the
|
|
||||||
// call sequence these tests pin. Stub it valid; the invalid path has its
|
|
||||||
// own test below.
|
|
||||||
jest.mock('../../../services/eventTypeService', () => ({
|
|
||||||
isValidEventType: jest.fn().mockResolvedValue(true),
|
|
||||||
}));
|
|
||||||
|
|
||||||
const { db } = require('../../../database/db');
|
const { db } = require('../../../database/db');
|
||||||
const { isValidEventType } = require('../../../services/eventTypeService');
|
|
||||||
const eventsRouter = require('../events');
|
const eventsRouter = require('../events');
|
||||||
|
|
||||||
const buildApp = () => {
|
const buildApp = () => {
|
||||||
@@ -240,16 +231,4 @@ describe('v1 POST /events — issue #550 (color_theme + feedback row)', () => {
|
|||||||
.send({ ...BASE_BODY, feedback_enabled: 'maybe' })
|
.send({ ...BASE_BODY, feedback_enabled: 'maybe' })
|
||||||
.expect(400);
|
.expect(400);
|
||||||
});
|
});
|
||||||
|
|
||||||
it('rejects an event_type unknown to the catalog with 400 (#800)', async () => {
|
|
||||||
isValidEventType.mockResolvedValueOnce(false);
|
|
||||||
const res = await request(buildApp())
|
|
||||||
.post('/events')
|
|
||||||
.send({ ...BASE_BODY, event_type: 'nope' })
|
|
||||||
.expect(400);
|
|
||||||
|
|
||||||
expect(isValidEventType).toHaveBeenCalledWith('nope');
|
|
||||||
expect(JSON.stringify(res.body.errors)).toContain('event_type');
|
|
||||||
expect(db).not.toHaveBeenCalled();
|
|
||||||
});
|
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -26,7 +26,6 @@ const logger = require('../../utils/logger');
|
|||||||
const { slugify } = require('../../utils/slug');
|
const { slugify } = require('../../utils/slug');
|
||||||
const { formatBoolean } = require('../../utils/dbCompat');
|
const { formatBoolean } = require('../../utils/dbCompat');
|
||||||
const { parseBooleanInput } = require('../../utils/parsers');
|
const { parseBooleanInput } = require('../../utils/parsers');
|
||||||
const { isValidEventType } = require('../../services/eventTypeService');
|
|
||||||
|
|
||||||
const router = express.Router();
|
const router = express.Router();
|
||||||
|
|
||||||
@@ -81,7 +80,7 @@ const photoUpload = multer({
|
|||||||
* event_name: { type: string }
|
* event_name: { type: string }
|
||||||
* event_type:
|
* event_type:
|
||||||
* type: string
|
* type: string
|
||||||
* description: "Slug of an active event type from the catalog (Settings → Event Types). Defaults on a fresh install: wedding, birthday, corporate, other. GET /api/v1/event-types lists the live values."
|
* enum: [wedding, birthday, corporate, other, family]
|
||||||
* event_date: { type: string, format: date, nullable: true }
|
* event_date: { type: string, format: date, nullable: true }
|
||||||
* customer_name: { type: string, nullable: true }
|
* customer_name: { type: string, nullable: true }
|
||||||
* customer_email: { type: string, format: email, nullable: true }
|
* customer_email: { type: string, format: email, nullable: true }
|
||||||
@@ -118,14 +117,7 @@ router.post(
|
|||||||
requireApiScope('admin'),
|
requireApiScope('admin'),
|
||||||
[
|
[
|
||||||
body('event_name').isString().trim().notEmpty(),
|
body('event_name').isString().trim().notEmpty(),
|
||||||
// Validate against the live event_types catalog (admins can rename/delete
|
body('event_type').isIn(['wedding', 'birthday', 'corporate', 'other', 'family']),
|
||||||
// the defaults and add custom types), not a hardcoded whitelist (#800).
|
|
||||||
body('event_type').isString().trim().notEmpty().bail().custom(async (value) => {
|
|
||||||
if (!(await isValidEventType(value))) {
|
|
||||||
throw new Error('Unknown event type — must match an active event type slug');
|
|
||||||
}
|
|
||||||
return true;
|
|
||||||
}),
|
|
||||||
body('event_date').optional({ nullable: true, checkFalsy: true }).isISO8601(),
|
body('event_date').optional({ nullable: true, checkFalsy: true }).isISO8601(),
|
||||||
body('customer_name').optional({ nullable: true }).isString(),
|
body('customer_name').optional({ nullable: true }).isString(),
|
||||||
body('customer_email').optional({ nullable: true, checkFalsy: true }).isEmail(),
|
body('customer_email').optional({ nullable: true, checkFalsy: true }).isEmail(),
|
||||||
@@ -455,48 +447,6 @@ router.get(
|
|||||||
}
|
}
|
||||||
);
|
);
|
||||||
|
|
||||||
// ──────────────────────────────────────────────────────────────────────────
|
|
||||||
// GET /event-types — read (catalog discovery for event creation, #800)
|
|
||||||
// ──────────────────────────────────────────────────────────────────────────
|
|
||||||
|
|
||||||
/**
|
|
||||||
* @openapi
|
|
||||||
* /event-types:
|
|
||||||
* get:
|
|
||||||
* tags: [Events]
|
|
||||||
* summary: List active event types
|
|
||||||
* description: The slugs accepted as `event_type` when creating events. The catalog is admin-customizable (Settings → Event Types), so integrations should discover values here instead of hardcoding them.
|
|
||||||
* security: [{ bearerAuth: [] }]
|
|
||||||
* responses:
|
|
||||||
* 200:
|
|
||||||
* description: Active event types
|
|
||||||
* content:
|
|
||||||
* application/json:
|
|
||||||
* schema:
|
|
||||||
* type: object
|
|
||||||
* properties:
|
|
||||||
* eventTypes:
|
|
||||||
* type: array
|
|
||||||
* items:
|
|
||||||
* type: object
|
|
||||||
* properties:
|
|
||||||
* slug_prefix: { type: string }
|
|
||||||
* name: { type: string }
|
|
||||||
* emoji: { type: string }
|
|
||||||
*/
|
|
||||||
router.get('/event-types', apiTokenAuth, requireApiScope('read'), async (req, res) => {
|
|
||||||
try {
|
|
||||||
const types = await db('event_types')
|
|
||||||
.where('is_active', formatBoolean(true))
|
|
||||||
.orderBy('display_order', 'asc')
|
|
||||||
.select('slug_prefix', 'name', 'emoji');
|
|
||||||
res.json({ eventTypes: types });
|
|
||||||
} catch (error) {
|
|
||||||
logger.error('v1 GET /event-types failed', { error: error.message });
|
|
||||||
res.status(500).json({ error: 'Failed to list event types' });
|
|
||||||
}
|
|
||||||
});
|
|
||||||
|
|
||||||
// ──────────────────────────────────────────────────────────────────────────
|
// ──────────────────────────────────────────────────────────────────────────
|
||||||
// GET /events/:id — read
|
// GET /events/:id — read
|
||||||
// ──────────────────────────────────────────────────────────────────────────
|
// ──────────────────────────────────────────────────────────────────────────
|
||||||
|
|||||||
@@ -11,7 +11,6 @@ const businessProfileService = require('../businessProfileService');
|
|||||||
const { ensureSystemBlocksSeeded } = require('../contractBlocksService');
|
const { ensureSystemBlocksSeeded } = require('../contractBlocksService');
|
||||||
const { ensureInt } = require('../../utils/numericHelpers');
|
const { ensureInt } = require('../../utils/numericHelpers');
|
||||||
const { adminActor, ensureCustomerActive, nextContractNumber } = require('./helpers');
|
const { adminActor, ensureCustomerActive, nextContractNumber } = require('./helpers');
|
||||||
const { resolveDefaultEventType } = require('../eventTypeService');
|
|
||||||
|
|
||||||
|
|
||||||
/**
|
/**
|
||||||
@@ -222,12 +221,6 @@ async function convertToEvent(contractId, adminId) {
|
|||||||
const placeholderHash = crypto.randomBytes(32).toString('hex');
|
const placeholderHash = crypto.randomBytes(32).toString('hex');
|
||||||
const shareToken = crypto.randomBytes(32).toString('hex');
|
const shareToken = crypto.randomBytes(32).toString('hex');
|
||||||
|
|
||||||
// Event type: the configurable org default, else the resolved catch-all —
|
|
||||||
// same chain as quoteService.convertToEvent. Never a hardcoded slug: the
|
|
||||||
// admin may have renamed or deleted 'wedding' (#800).
|
|
||||||
const eventType = (await getAppSetting('crm_default_event_type'))
|
|
||||||
|| (await resolveDefaultEventType());
|
|
||||||
|
|
||||||
const eventCols = await db('events').columnInfo();
|
const eventCols = await db('events').columnInfo();
|
||||||
const candidate = {
|
const candidate = {
|
||||||
slug: `contract-${contract.contract_number.toLowerCase()}-${crypto.randomBytes(3).toString('hex')}`,
|
slug: `contract-${contract.contract_number.toLowerCase()}-${crypto.randomBytes(3).toString('hex')}`,
|
||||||
@@ -243,7 +236,7 @@ async function convertToEvent(contractId, adminId) {
|
|||||||
customer_email: customerEmail,
|
customer_email: customerEmail,
|
||||||
customer_phone: customer.phone,
|
customer_phone: customer.phone,
|
||||||
admin_email: adminEmail,
|
admin_email: adminEmail,
|
||||||
event_type: eventType,
|
event_type: 'wedding',
|
||||||
password_hash: placeholderHash,
|
password_hash: placeholderHash,
|
||||||
share_link: shareToken,
|
share_link: shareToken,
|
||||||
share_token: shareToken,
|
share_token: shareToken,
|
||||||
|
|||||||
@@ -428,28 +428,6 @@ const getEventTypeForSlug = async (eventTypeIdentifier) => {
|
|||||||
return { slug_prefix: 'event', theme_preset: 'default', emoji: '📷' };
|
return { slug_prefix: 'event', theme_preset: 'default', emoji: '📷' };
|
||||||
};
|
};
|
||||||
|
|
||||||
/**
|
|
||||||
* Resolve the fallback event type for document→event conversions (quotes,
|
|
||||||
* contracts) when the source carries none. Never hardcodes a specific slug
|
|
||||||
* (any of them, incl. 'other', can be disabled or deleted by the admin):
|
|
||||||
* prefer the generic 'other' catch-all when it's active, else the first
|
|
||||||
* active type by display order, and only fall back to the literal 'other'
|
|
||||||
* if the catalog is somehow empty/unreadable.
|
|
||||||
* @param {Object} [conn] - Optional knex connection/transaction
|
|
||||||
* @returns {Promise<string>} - slug_prefix to use
|
|
||||||
*/
|
|
||||||
const resolveDefaultEventType = async (conn) => {
|
|
||||||
const q = conn || db;
|
|
||||||
try {
|
|
||||||
const other = await q('event_types').where({ slug_prefix: 'other', is_active: formatBoolean(true) }).first('slug_prefix');
|
|
||||||
if (other) return 'other';
|
|
||||||
const firstActive = await q('event_types').where({ is_active: formatBoolean(true) }).orderBy('display_order', 'asc').first('slug_prefix');
|
|
||||||
return firstActive?.slug_prefix || 'other';
|
|
||||||
} catch (_) {
|
|
||||||
return 'other';
|
|
||||||
}
|
|
||||||
};
|
|
||||||
|
|
||||||
module.exports = {
|
module.exports = {
|
||||||
getAllEventTypes,
|
getAllEventTypes,
|
||||||
getActiveEventTypes,
|
getActiveEventTypes,
|
||||||
@@ -461,6 +439,5 @@ module.exports = {
|
|||||||
updateEventType,
|
updateEventType,
|
||||||
deleteEventType,
|
deleteEventType,
|
||||||
reorderEventTypes,
|
reorderEventTypes,
|
||||||
getEventTypeForSlug,
|
getEventTypeForSlug
|
||||||
resolveDefaultEventType
|
|
||||||
};
|
};
|
||||||
|
|||||||
@@ -34,7 +34,6 @@ const { cleanNetMinor } = require('../utils/invoiceRounding');
|
|||||||
const { AppError } = require('../utils/errors');
|
const { AppError } = require('../utils/errors');
|
||||||
const { formatBoolean } = require('../utils/dbCompat');
|
const { formatBoolean } = require('../utils/dbCompat');
|
||||||
const { nextDocumentNumber } = require('../utils/documentSequences');
|
const { nextDocumentNumber } = require('../utils/documentSequences');
|
||||||
const { resolveDefaultEventType } = require('./eventTypeService');
|
|
||||||
const { formatShortDate } = require('../utils/dateFormatter');
|
const { formatShortDate } = require('../utils/dateFormatter');
|
||||||
const businessProfileService = require('./businessProfileService');
|
const businessProfileService = require('./businessProfileService');
|
||||||
const { buildIssuerBlock, buildRecipientBlock } = require('./_renderContext');
|
const { buildIssuerBlock, buildRecipientBlock } = require('./_renderContext');
|
||||||
@@ -309,6 +308,25 @@ async function nextQuoteNumber(trx) {
|
|||||||
return nextDocumentNumber('quote', 'crm_quotes_number_format', 'Q-{YEAR}-{SEQ:04d}', trx);
|
return nextDocumentNumber('quote', 'crm_quotes_number_format', 'Q-{YEAR}-{SEQ:04d}', trx);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Resolve the fallback event type for a quote→event conversion when the quote
|
||||||
|
* itself carries none. Never hardcodes a specific slug (any of them, incl.
|
||||||
|
* 'other', can be disabled by the admin): prefer the generic 'other' catch-all
|
||||||
|
* when it's active, else the first active type by display order, and only fall
|
||||||
|
* back to the literal 'other' if the catalog is somehow empty/unreadable.
|
||||||
|
*/
|
||||||
|
async function resolveDefaultEventType(conn) {
|
||||||
|
const q = conn || db;
|
||||||
|
try {
|
||||||
|
const other = await q('event_types').where({ slug_prefix: 'other', is_active: true }).first('slug_prefix');
|
||||||
|
if (other) return 'other';
|
||||||
|
const firstActive = await q('event_types').where({ is_active: true }).orderBy('display_order', 'asc').first('slug_prefix');
|
||||||
|
return firstActive?.slug_prefix || 'other';
|
||||||
|
} catch (_) {
|
||||||
|
return 'other';
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
function ensureCustomerFeatureEnabled(customer, feature) {
|
function ensureCustomerFeatureEnabled(customer, feature) {
|
||||||
// Global toggle (`customer_feature_quotes_enabled` / `..._bills_enabled`)
|
// Global toggle (`customer_feature_quotes_enabled` / `..._bills_enabled`)
|
||||||
// is checked at the route layer (feature flag); here we only enforce
|
// is checked at the route layer (feature flag); here we only enforce
|
||||||
|
|||||||
Reference in New Issue
Block a user