diff --git a/ghost/core/server/services/README.md b/ghost/core/server/services/README.md index 602b45b5dcfc..6a194e3125c8 100644 --- a/ghost/core/server/services/README.md +++ b/ghost/core/server/services/README.md @@ -49,7 +49,7 @@ For example: implementation through `init()` and `service`. It qualifies without owning a background worker. - Staff invitation acceptance is a domain action owned by `invites`. Its - `accept.js` function keeps transaction state local to each invocation; it + `accept.ts` function keeps transaction state local to each invocation; it does not need its own service root or lifecycle. - Magic-link token and URL helpers are reusable support for several workflows, not a separately boot-managed capability. They belong in their own library, diff --git a/ghost/core/server/services/invites/accept.js b/ghost/core/server/services/invites/accept.ts similarity index 51% rename from ghost/core/server/services/invites/accept.js rename to ghost/core/server/services/invites/accept.ts index 77a7eb9b06d9..f6b01a37fe00 100644 --- a/ghost/core/server/services/invites/accept.js +++ b/ghost/core/server/services/invites/accept.ts @@ -1,6 +1,40 @@ -const errors = require('@tryghost/errors'); -const tpl = require('@tryghost/tpl'); -const security = require('@tryghost/security'); +import type { Knex } from 'knex'; +import * as errors from '@tryghost/errors'; +import tpl from '@tryghost/tpl'; +import * as security from '@tryghost/security'; + +export type Invitation = { + invitation: Array<{ token: string; name: string; password: string }>; +}; + +type TransactionOptions = { + context: { internal: true }; + transacting: Transaction; + forUpdate: true; +}; + +export type AcceptanceModels = { + Base: { + transaction(callback: (transacting: Transaction) => Promise): Promise; + }; + Invite: { + findOne( + data: { token: string; status: 'sent' }, + options: TransactionOptions, + ): Promise<{ + get(key: 'expires'): number; + get(key: 'email' | 'role_id'): string; + destroy(options: TransactionOptions): Promise; + } | null>; + }; + User: { + findOne(data: { email: string }, options: TransactionOptions): Promise; + add( + data: { email: string; name: string; password: string; roles: string[] }, + options: TransactionOptions, + ): Promise; + }; +}; const messages = { inviteNotFound: 'Invite not found.', @@ -12,12 +46,19 @@ const messages = { }, }; -async function accept(invitation, models) { +export async function accept( + invitation: Invitation, + models: AcceptanceModels, +): Promise { const data = invitation.invitation[0]; const inviteToken = security.url.decodeBase64(data.token); return models.Base.transaction(async (transacting) => { - const options = { context: { internal: true }, transacting, forUpdate: true }; + const options: TransactionOptions = { + context: { internal: true }, + transacting, + forUpdate: true, + }; const invite = await models.Invite.findOne({ token: inviteToken, status: 'sent' }, options); @@ -55,5 +96,3 @@ async function accept(invitation, models) { ); }); } - -module.exports = accept; diff --git a/ghost/core/server/services/invites/index.ts b/ghost/core/server/services/invites/index.ts index a8d877f0436b..6dbb81991475 100644 --- a/ghost/core/server/services/invites/index.ts +++ b/ghost/core/server/services/invites/index.ts @@ -1,6 +1,5 @@ import { defineService } from '../../../kernel/define-service'; -// @ts-expect-error The existing CommonJS implementation has no type declarations. -import Invites from './invites'; +import { Invites } from './invites'; export default defineService('Invites', () => { const settingsCache = require('../../../shared/settings-cache'); diff --git a/ghost/core/server/services/invites/invites.js b/ghost/core/server/services/invites/invites.ts similarity index 54% rename from ghost/core/server/services/invites/invites.js rename to ghost/core/server/services/invites/invites.ts index 0b3b8f7df762..68d04f7fffb8 100644 --- a/ghost/core/server/services/invites/invites.js +++ b/ghost/core/server/services/invites/invites.ts @@ -1,6 +1,73 @@ -const security = require('@tryghost/security'); -const tpl = require('@tryghost/tpl'); -const accept = require('./accept'); +import * as security from '@tryghost/security'; +import tpl from '@tryghost/tpl'; +import { accept, type AcceptanceModels, type Invitation } from './accept'; + +type Invite = { + id: string; + get(key: 'token' | 'email'): string; +}; + +type InviteData = { email: string; role_id: string }; +type InviteOptions = Record; +type InviteEmailData = { + blogName: string; + invitedByName?: string; + invitedByEmail?: string; + resetLink: string; + recipientEmail: string; +}; + +type Dependencies = { + settingsCache: { get(key: 'title'): string }; + settingsHelpers: { getDefaultEmailDomain(): string }; + mailService: { + utils: { + generateContent(options: { + data: InviteEmailData; + template: 'invite-user' | 'invite-user-by-api-key'; + }): Promise<{ html: string; text: string }>; + }; + }; + urlUtils: { + urlFor(context: 'admin', absolute: true): string; + urlJoin(...parts: string[]): string; + }; + models: AcceptanceModels; + logging: { warn(message: string): void }; +}; + +type AddInviteRequest = { + api: { + mail: { + send( + payload: { + mail: Array<{ + message: { + to: string; + replyTo?: string; + subject: string; + html: string; + text: string; + }; + options: Record; + }>; + }, + options: { context: { internal: true } }, + ): Promise; + }; + }; + InviteModel: { + findOne( + data: { email: string }, + options: InviteOptions, + ): Promise<{ destroy(options: InviteOptions): Promise } | null>; + add(data: InviteData, options: InviteOptions): Promise; + edit(data: { status: 'sent' }, options: InviteOptions & { id: string }): Promise; + }; + invites: InviteData[]; + options: InviteOptions; + user: { name?: string; email?: string }; +}; const messages = { invitedByName: '{invitedByName} has invited you to join {blogName}', @@ -11,8 +78,22 @@ const messages = { }, }; -class Invites { - constructor({ settingsCache, settingsHelpers, mailService, urlUtils, models, logging }) { +export class Invites { + private readonly settingsCache: Dependencies['settingsCache']; + private readonly settingsHelpers: Dependencies['settingsHelpers']; + private readonly mailService: Dependencies['mailService']; + private readonly urlUtils: Dependencies['urlUtils']; + private readonly models: Dependencies['models']; + private readonly logging: Dependencies['logging']; + + constructor({ + settingsCache, + settingsHelpers, + mailService, + urlUtils, + models, + logging, + }: Dependencies) { this.settingsCache = settingsCache; this.settingsHelpers = settingsHelpers; this.mailService = mailService; @@ -21,13 +102,13 @@ class Invites { this.logging = logging; } - accept(invitation) { + accept(invitation: Invitation) { return accept(invitation, this.models); } - add({ api, InviteModel, invites, options, user }) { - let invite; - let emailData; + add({ api, InviteModel, invites, options, user }: AddInviteRequest): Promise { + let invite: Invite; + let emailData: InviteEmailData; return InviteModel.findOne({ email: invites[0].email }, options) .then((existingInvite) => { @@ -118,19 +199,24 @@ class Invites { .then((editedInvite) => { return editedInvite; }) - .catch((err) => { - if (err && err.errorType === 'EmailError') { + .catch((err: unknown) => { + if ( + err && + typeof err === 'object' && + 'errorType' in err && + err.errorType === 'EmailError' && + 'message' in err + ) { const errorMessage = tpl(messages.errorSendingEmail.error, { message: err.message, }); const helpText = tpl(messages.errorSendingEmail.help); - err.message = `${errorMessage} ${helpText}`; - this.logging.warn(err.message); + const message = `${errorMessage} ${helpText}`; + err.message = message; + this.logging.warn(message); } return Promise.reject(err); }); } } - -module.exports = Invites; diff --git a/ghost/core/server/services/service-inventory.yaml b/ghost/core/server/services/service-inventory.yaml index a430be288c7b..716cefe14c01 100644 --- a/ghost/core/server/services/service-inventory.yaml +++ b/ghost/core/server/services/service-inventory.yaml @@ -5514,8 +5514,8 @@ services: reviewedAt: 61ac1e5058fa4657def92d7b362e197314b1a88d sources: - core/server/services/invites/index.ts - - core/server/services/invites/invites.js - - core/server/services/invites/accept.js + - core/server/services/invites/invites.ts + - core/server/services/invites/accept.ts - core/server/api/endpoints/invites.js - core/server/api/endpoints/authentication.js dependencies: @@ -5537,8 +5537,8 @@ services: reviewedAt: 44ad405bff4a8700af2ed51f833ebc08d25dddff sources: - core/server/services/invites/index.ts - - core/server/services/invites/invites.js - - core/server/services/invites/accept.js + - core/server/services/invites/invites.ts + - core/server/services/invites/accept.ts - core/server/api/endpoints/invites.js - core/server/api/endpoints/authentication.js entryPoints: @@ -5570,10 +5570,10 @@ services: The TypeScript root default-exports defineService with init and the guarded service getter, with no extras or other runtime exports. CommonJS callers read default. The ready service is one Invites instance whose add method needs its receiver; it retains - mutable settingsCache, settingsHelpers, mailService, urlUtils, models and logging fields. - The deep invites.js module still exports the constructor. There is no shutdown or reset; + settingsCache, settingsHelpers, mailService, urlUtils, models and logging facades. + The deep invites.ts module exports the named Invites constructor. There is no shutdown or reset; add resolves the edited Invite model. The instance also exposes accept, delegating to - the CommonJS accept.js action + the named accept.ts action with its injected models. It resolves undefined on success; the API serializer supplies the success message. The action adds no application instance, init or shutdown. Its former lazy barrel is preserved at @@ -5583,8 +5583,8 @@ services: sources: - core/kernel/define-service.ts - core/server/services/invites/index.ts - - core/server/services/invites/invites.js - - core/server/services/invites/accept.js + - core/server/services/invites/invites.ts + - core/server/services/invites/accept.ts - core/server/api/endpoints/utils/serializers/output/authentication.js tests: - test/unit/server/services/invites/index.test.ts @@ -5608,14 +5608,14 @@ services: sources: - core/boot.js - core/server/services/invites/index.ts - - core/server/services/invites/invites.js + - core/server/services/invites/invites.ts - core/server/lib/mail/index.js - - core/server/services/invites/accept.js + - core/server/services/invites/accept.ts - core/server/models/base/index.js - core/server/models/base/plugins/events.js - core/server/models/user.js tests: - - test/unit/server/services/invites/accept.test.js + - test/unit/server/services/invites/accept.test.ts - test/unit/server/services/invites/index.test.ts consumers: summary: >- @@ -5627,7 +5627,7 @@ services: Root units separately assert staff/integration template data and the exact API-mail payload, with controlled model and rendering collaborators. The authentication endpoint reads service.accept from the retained root after input - validation and setup checks; that method forwards to accept.js with the same models. + validation and setup checks; that method forwards to accept.ts with the same models. Its GET invitation lookup queries models directly. Acceptance units import the function with explicit models; legacy HTTP cases exercise the root's delegation through the real endpoint, asserting @@ -5643,7 +5643,7 @@ services: - test/e2e-api/admin/invites.test.js - test/e2e-api/admin/host-limits.test.ts - test/unit/server/models/invite.test.js - - test/unit/server/services/invites/accept.test.js + - test/unit/server/services/invites/accept.test.ts - test/legacy/api/admin/authentication.test.js - test/unit/server/services/invites/index.test.ts startup: @@ -5665,7 +5665,7 @@ services: - core/server/services/invites/index.ts - core/server/api/endpoints/mail.js - core/server/lib/mail/ghost-mailer.js - - core/server/services/invites/accept.js + - core/server/services/invites/accept.ts - core/server/api/endpoints/authentication.js - core/server/api/endpoints/utils/validators/input/invitations.js tests: @@ -5687,9 +5687,9 @@ services: token reuse or failed creation leaving the invite available. sources: - core/kernel/define-service.ts - - core/server/services/invites/invites.js + - core/server/services/invites/invites.ts - core/server/api/endpoints/mail.js - - core/server/services/invites/accept.js + - core/server/services/invites/accept.ts tests: - test/unit/kernel/define-service.test.ts - test/e2e-api/admin/invites.test.js @@ -5708,17 +5708,17 @@ services: configuration snapshot. Acceptance units pin URL-safe token decoding, rejection one millisecond after expiry and acceptance at the exact expiry boundary using a fixed clock. sources: - - core/server/services/invites/invites.js + - core/server/services/invites/invites.ts - core/server/models/invite.js - core/server/lib/mail/email-content-generator.js - core/server/lib/mail/ghost-mailer.js - core/server/api/endpoints/mail.js - - core/server/services/invites/accept.js + - core/server/services/invites/accept.ts - core/server/models/user.js - core/server/data/db/index.js tests: - test/unit/server/services/invites/index.test.ts - - test/unit/server/services/invites/accept.test.js + - test/unit/server/services/invites/accept.test.ts resources: summary: >- The root borrows models, template filesystem work and API-mail sending. It awaits @@ -5734,17 +5734,17 @@ services: prove deletion finishes before user creation and acceptance waits for creation. They do not test actual rollback, acquired locks or committed event delivery. sources: - - core/server/services/invites/invites.js + - core/server/services/invites/invites.ts - core/server/api/endpoints/mail.js - core/server/lib/mail/ghost-mailer.js - core/server/lib/mail/email-content-generator.js - core/server/ghost-server.ts - - core/server/services/invites/accept.js + - core/server/services/invites/accept.ts - core/server/models/user.js - core/server/models/base/bookshelf.js - core/server/models/base/plugins/events.js tests: - - test/unit/server/services/invites/accept.test.js + - test/unit/server/services/invites/accept.test.ts - test/unit/server/services/invites/index.test.ts failures: summary: >- @@ -5767,17 +5767,17 @@ services: after deletion, emitted events and persisted staff-role assignment remain untested here. sources: - core/kernel/define-service.ts - - core/server/services/invites/invites.js + - core/server/services/invites/invites.ts - core/server/models/invite.js - core/server/api/endpoints/mail.js - - core/server/services/invites/accept.js + - core/server/services/invites/accept.ts - core/server/models/base/index.js - core/server/api/endpoints/utils/validators/input/invitations.js tests: - test/unit/kernel/define-service.test.ts - test/legacy/api/admin/authentication.test.js - test/unit/server/services/invites/index.test.ts - - test/unit/server/services/invites/accept.test.js + - test/unit/server/services/invites/accept.test.ts unverified: [] blockers: - >- @@ -12279,7 +12279,7 @@ services: - core/server/services/staff/staff-service-emails.js - core/server/services/gifts/index.ts - core/server/services/invites/index.ts - - core/server/services/invites/invites.js + - core/server/services/invites/invites.ts - core/server/services/email-address/email-address-service-wrapper.js - core/server/services/posts/posts-service-instance.js - core/server/services/posts/posts-exporter.js diff --git a/ghost/test/unit/server/services/invites/accept.test.js b/ghost/test/unit/server/services/invites/accept.test.ts similarity index 82% rename from ghost/test/unit/server/services/invites/accept.test.js rename to ghost/test/unit/server/services/invites/accept.test.ts index 8f9fd044e338..02ed92e05f9a 100644 --- a/ghost/test/unit/server/services/invites/accept.test.js +++ b/ghost/test/unit/server/services/invites/accept.test.ts @@ -1,19 +1,27 @@ -const assert = require('node:assert/strict'); -const { setImmediate } = require('node:timers/promises'); -const sinon = require('sinon'); -const accept = require('../../../../../core/server/services/invites/accept'); -const models = require('../../../../../core/server/models'); +import assert from 'node:assert/strict'; +import { setImmediate } from 'node:timers/promises'; +import sinon from 'sinon'; +import { accept, type AcceptanceModels } from '../../../../../core/server/services/invites/accept'; describe('Unit: services/invites/accept', function () { const now = 1700000000000; const transacting = { id: 'fake-txn' }; - let transactionStub; - let findOneInviteStub; - let findOneUserStub; - let destroyStub; - let addUserStub; - let inviteAttributes; - let invitation; + let models: AcceptanceModels; + let transactionStub: sinon.SinonStub; + let findOneInviteStub: sinon.SinonStub; + let findOneUserStub: sinon.SinonStub; + let destroyStub: sinon.SinonStub; + let addUserStub: sinon.SinonStub; + let inviteAttributes: { expires: number; email: string; role_id: string }; + let invitation: { + invitation: Array<{ + token: string; + email: string; + role_id: string; + name: string; + password: string; + }>; + }; beforeEach(function () { sinon.stub(Date, 'now').returns(now); @@ -35,18 +43,21 @@ describe('Unit: services/invites/accept', function () { }; destroyStub = sinon.stub().resolves(); findOneInviteStub = sinon.stub().resolves({ - get: (key) => inviteAttributes[key], + get: (key: Key) => inviteAttributes[key], destroy: destroyStub, }); findOneUserStub = sinon.stub().resolves(null); addUserStub = sinon.stub().resolves(); // This fixture drives transaction propagation; it does not simulate database rollback. - transactionStub = sinon.stub().callsFake(async (cb) => cb(transacting)); - - sinon.stub(models.Base, 'transaction').callsFake(transactionStub); - sinon.stub(models.Invite, 'findOne').callsFake(findOneInviteStub); - sinon.stub(models.User, 'findOne').callsFake(findOneUserStub); - sinon.stub(models.User, 'add').callsFake(addUserStub); + transactionStub = sinon + .stub() + .callsFake(async (cb: (transaction: typeof transacting) => Promise) => cb(transacting)); + + models = { + Base: { transaction: transactionStub }, + Invite: { findOne: findOneInviteStub }, + User: { findOne: findOneUserStub, add: addUserStub }, + }; }); afterEach(function () { @@ -75,7 +86,7 @@ describe('Unit: services/invites/accept', function () { }); it('destroys invite before creating user', async function () { - const callOrder = []; + const callOrder: string[] = []; destroyStub.callsFake(async () => { callOrder.push('destroy'); }); @@ -180,8 +191,8 @@ describe('Unit: services/invites/accept', function () { }); it('waits for invite deletion to finish before creating the user', async function () { - const deletion = Promise.withResolvers(); - const started = Promise.withResolvers(); + const deletion = Promise.withResolvers(); + const started = Promise.withResolvers(); destroyStub.callsFake(() => { started.resolve(); return deletion.promise; @@ -203,8 +214,8 @@ describe('Unit: services/invites/accept', function () { }); it('waits for user creation before resolving acceptance', async function () { - const creation = Promise.withResolvers(); - const started = Promise.withResolvers(); + const creation = Promise.withResolvers<{ id: string }>(); + const started = Promise.withResolvers(); let settled = false; addUserStub.callsFake(() => { started.resolve(); @@ -259,10 +270,12 @@ describe('Unit: services/invites/accept', function () { it('propagates transaction failure after the callback has completed', async function () { const failure = new Error('Transaction completion failed'); - transactionStub.callsFake(async (callback) => { - await callback(transacting); - throw failure; - }); + transactionStub.callsFake( + async (callback: (transaction: typeof transacting) => Promise) => { + await callback(transacting); + throw failure; + }, + ); await assert.rejects(accept(invitation, models), (error) => error === failure); diff --git a/ghost/test/unit/server/services/invites/index.test.ts b/ghost/test/unit/server/services/invites/index.test.ts index 7a7b0e5565bb..0f842024e11b 100644 --- a/ghost/test/unit/server/services/invites/index.test.ts +++ b/ghost/test/unit/server/services/invites/index.test.ts @@ -2,6 +2,7 @@ import assert from 'node:assert/strict'; import sinon from 'sinon'; const errors = require('@tryghost/errors'); const logging = require('@tryghost/logging'); +import type { Invites } from '../../../../../core/server/services/invites/invites'; const rootPath = require.resolve('../../../../../core/server/services/invites'); const settingsCache = require('../../../../../core/shared/settings-cache'); @@ -12,18 +13,12 @@ const urlUtils = require('../../../../../core/shared/url-utils').default; describe('Invites service root', function () { const sandbox = sinon.createSandbox(); let previousModule: NodeJS.Module | undefined; - let invitesService: { add(request: typeof input): Promise }; + let invitesService: Invites; let InviteModel: { findOne: sinon.SinonStub; add: sinon.SinonStub; edit: sinon.SinonStub }; let api: { mail: { send: sinon.SinonStub } }; - let input: { - api: typeof api; - InviteModel: typeof InviteModel; - invites: Array<{ email: string; role_id: string }>; - options: { context: { user: string } }; - user: { name?: string; email?: string }; - }; - let created: { id: string; get(key: string): string }; - let edited: { id: string; status: string }; + let input: Parameters[0]; + let created: Awaited>; + let edited: Awaited> & { status: string }; let render: sinon.SinonStub; let getTitle: sinon.SinonStub; let getDomain: sinon.SinonStub; @@ -38,7 +33,7 @@ describe('Invites service root', function () { email: 'recipient@example.com', }; created = { id: 'invite-id', get: (key) => values[key] }; - edited = { id: 'invite-id', status: 'sent' }; + edited = { ...created, status: 'sent' }; InviteModel = { findOne: sandbox.stub().resolves(null), add: sandbox.stub().resolves(created),