diff --git a/backend/__tests__/integration/productUsagePg.test.js b/backend/__tests__/integration/productUsagePg.test.js index 34fa206f..ea4e3835 100644 --- a/backend/__tests__/integration/productUsagePg.test.js +++ b/backend/__tests__/integration/productUsagePg.test.js @@ -27,12 +27,25 @@ maybe('product usage on Postgres', () => { let UsageService; beforeAll(async () => { - db = knex({ client: 'pg', connection: PG_URL, pool: { min: 0, max: 10 } }); - for (const t of ['product_usage_markers', 'product_usage_state', 'app_settings', - 'feature_flags', 'events', 'css_templates', 'email_configs', - 'mail_accounts', 'whatsapp_configs']) { - await db.raw(`DROP TABLE IF EXISTS ${t} CASCADE`); - } + // Its own schema, not `public`. CI hands every gated suite the same + // PICPEAK_PG_TEST_URL and runs jest with parallel workers, and both + // picpeakRestorePg and externalRelpathFoldPg drop and recreate `events` + // and `app_settings` there. Sharing that would have made all three + // intermittently destroy each other's fixtures. The service queries + // unqualified table names, so a searchPath keeps it entirely in here. + const bootstrap = knex({ + client: 'pg', connection: PG_URL, pool: { min: 0, max: 2 } + }); + await bootstrap.raw('DROP SCHEMA IF EXISTS usage_pg_test CASCADE'); + await bootstrap.raw('CREATE SCHEMA usage_pg_test'); + await bootstrap.destroy(); + + db = knex({ + client: 'pg', + connection: PG_URL, + searchPath: ['usage_pg_test'], + pool: { min: 0, max: 10 } + }); // The real migrations, on the real engine. await require('../../migrations/core/201_product_usage').up(db); await require('../../migrations/core/202_product_usage_cancel_requested').up(db); @@ -61,7 +74,10 @@ maybe('product usage on Postgres', () => { }, 120000); afterAll(async () => { - if (db) await db.destroy(); + if (db) { + await db.raw('DROP SCHEMA IF EXISTS usage_pg_test CASCADE'); + await db.destroy(); + } fs.rmSync(bindingDir, { recursive: true, force: true }); }); diff --git a/backend/src/middleware/productUsage.js b/backend/src/middleware/productUsage.js index a8d41c14..853a74e5 100644 --- a/backend/src/middleware/productUsage.js +++ b/backend/src/middleware/productUsage.js @@ -3,6 +3,14 @@ // identifiers, paths, timing, or counts are retained or sent. const service = require('../services/productUsageService'); const logger = require('../utils/logger'); +// Mirrors emailWebhookTransport: the webhook is in play only when both are +// set, which is when adminEmail routes the test send through it. +const webhookTransportConfigured = () => + Boolean( + (process.env.EMAIL_WEBHOOK_URL || '').trim() && + (process.env.EMAIL_WEBHOOK_SECRET || '').trim() + ); + const RULES = [ [/^\/customers(?:\/|$)/, ['crm']], [/^\/quotes(?:\/|$)/, ['crm', 'crm_quotes']], @@ -38,9 +46,14 @@ function productUsage(req, res, next) { const pathname = req.path; res.once('finish', () => { if (!req.admin?.id || res.statusCode < 200 || res.statusCode >= 300) return; - const features = RULES.filter(([pattern]) => + let features = RULES.filter(([pattern]) => pattern.test(pathname) ).flatMap(([, keys]) => keys); + // A webhook-only install sends /email/test through the webhook transport + // and never touches SMTP (adminEmail.js has an explicit path for it, + // #1225), so recording smtp here would permanently misclassify it. + if (features.includes('smtp') && webhookTransportConfigured()) + features = features.filter((f) => f !== 'smtp'); if ( process.env.STORAGE_BACKEND === 's3' && /^\/(?:photos|events)\/[^/]+\/upload(?:\/|$)/.test(pathname) diff --git a/backend/src/usage/UsageService.js b/backend/src/usage/UsageService.js index c87584e7..2e8bd11f 100644 --- a/backend/src/usage/UsageService.js +++ b/backend/src/usage/UsageService.js @@ -474,17 +474,26 @@ class UsageService { update.last_packet = JSON.stringify(envelope); update.last_report_date = packet.payload.report_date; } - await this.db('product_usage_state') - .where({ id: 1 }) - .whereNot({ status: 'deletion_pending' }) - .update(update); + // Activation goes in with the acknowledgement, not after it. Split + // across two writes, a failure or a stop between them left the row + // `activation_pending` with pending_packet already cleared — and + // tick() has nothing to retry from there, so the installation was + // registered with the collector but permanently stuck locally. + // Still guarded on activation_pending, so a withdrawal that arrived + // first is not overwritten. + const ack = this.db('product_usage_state').where({ id: 1 }); + if (packet.action === 'register') { + update.status = 'active'; + // Precisely activation_pending, not merely "not withdrawing" — + // this write is the one that turns participation on. + ack.where({ status: 'activation_pending' }); + } else { + ack.whereNot({ status: 'deletion_pending' }); + } + await ack.update(update); await this.db('product_usage_state') .where({ id: 1, status: 'deletion_pending' }) .update({ sequence: packet.sequence, pending_packet: null }); - if (packet.action === 'register') - await this.db('product_usage_state') - .where({ id: 1, status: 'activation_pending' }) - .update({ status: 'active' }); } return receipt; } catch (error) {