Skip to content

Conversation

@cmraible
Copy link
Collaborator

@cmraible cmraible commented Nov 20, 2025

ref https://linear.app/ghost/issue/NY-771

This model enables CRUD operations on the automated_emails table, which will store customizable welcome email content for new members. Includes URL transformation for the lexical field to handle GHOST_URL placeholders consistently with other models like posts.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 20, 2025

Walkthrough

This pull request introduces a new Bookshelf model for AutomatedEmail, adding support for managing automated emails with a default inactive status. The model includes parse and formatOnWrite hooks to handle lexical URL transformations during storage and retrieval operations. A comprehensive unit test file validates the model's defaults behavior, URL parsing functionality, and URL formatting on write operations.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~15 minutes

  • Parse and formatOnWrite hook implementations: Verify that lexical URL transformations correctly convert between absolute URLs and __GHOST_URL__ placeholders, and that null/undefined cases are properly handled
  • Test coverage: Confirm that the unit tests adequately validate edge cases for the URL transformation logic and that field preservation is working as expected
  • Default status initialization: Ensure the default 'inactive' status is correctly configured in the model

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title 'Added automated_email bookshelf model' directly and accurately summarizes the main change: adding a new Bookshelf model for the automated_emails table.
Description check ✅ Passed The description explains the purpose of the model (CRUD operations on automated_emails table), its use case (storing customizable welcome email content), and the URL transformation implementation for lexical fields, all of which align with the changeset.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch chris-ny-771-add-automated_email-bookshelf-model

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5e5020c and 436338c.

📒 Files selected for processing (2)
  • ghost/core/core/server/models/automated-email.js (1 hunks)
  • ghost/core/test/unit/server/models/automated-email.test.js (1 hunks)
🧰 Additional context used
🧠 Learnings (7)
📚 Learning: 2025-11-25T14:28:50.351Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T14:28:50.351Z
Learning: Database schema should be defined in `ghost/core/core/server/data/schema/`

Applied to files:

  • ghost/core/core/server/models/automated-email.js
📚 Learning: 2025-03-13T09:00:20.205Z
Learnt from: mike182uk
Repo: TryGhost/Ghost PR: 22471
File: apps/admin-x-activitypub/src/utils/pending-activity.ts:13-71
Timestamp: 2025-03-13T09:00:20.205Z
Learning: The pending activity utilities in the Ghost ActivityPub module are covered by tests in the file `apps/admin-x-activitypub/test/unit/utils/pending-activity.ts`.

Applied to files:

  • ghost/core/test/unit/server/models/automated-email.test.js
📚 Learning: 2025-03-13T09:00:20.205Z
Learnt from: mike182uk
Repo: TryGhost/Ghost PR: 22471
File: apps/admin-x-activitypub/src/utils/pending-activity.ts:13-71
Timestamp: 2025-03-13T09:00:20.205Z
Learning: The pending activity utilities in Ghost's ActivityPub module are thoroughly tested in `apps/admin-x-activitypub/test/unit/utils/pending-activity.ts`, which covers `generatePendingActivityId`, `isPendingActivity`, and `generatePendingActivity` functions.

Applied to files:

  • ghost/core/test/unit/server/models/automated-email.test.js
📚 Learning: 2025-11-24T17:29:43.865Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: e2e/AGENTS.md:0-0
Timestamp: 2025-11-24T17:29:43.865Z
Learning: Applies to e2e/**/*.test.ts : Test suite names should follow the format 'Ghost Admin - Feature' or 'Ghost Public - Feature'

Applied to files:

  • ghost/core/test/unit/server/models/automated-email.test.js
📚 Learning: 2025-11-24T17:29:43.865Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: e2e/AGENTS.md:0-0
Timestamp: 2025-11-24T17:29:43.865Z
Learning: Each test receives fresh Ghost instance for automatic isolation

Applied to files:

  • ghost/core/test/unit/server/models/automated-email.test.js
📚 Learning: 2025-11-24T17:29:43.865Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: e2e/AGENTS.md:0-0
Timestamp: 2025-11-24T17:29:43.865Z
Learning: Applies to e2e/**/*.test.ts : Follow the AAA (Arrange, Act, Assert) pattern in test structure

Applied to files:

  • ghost/core/test/unit/server/models/automated-email.test.js
📚 Learning: 2025-05-29T07:45:35.714Z
Learnt from: ErisDS
Repo: TryGhost/Ghost PR: 23582
File: ghost/core/.c8rc.json:24-24
Timestamp: 2025-05-29T07:45:35.714Z
Learning: In Ghost project, app.js files under core/server/web are intentionally excluded from unit test coverage because they are not easily unit-testable due to being entry points with initialization code and side effects.

Applied to files:

  • ghost/core/test/unit/server/models/automated-email.test.js
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (12)
  • GitHub Check: E2E Tests (Ember 1/2)
  • GitHub Check: E2E Tests (Ember 2/2)
  • GitHub Check: [Optional] E2E Tests (React 2/2)
  • GitHub Check: [Optional] E2E Tests (React 1/2)
  • GitHub Check: Inspect Docker Image
  • GitHub Check: Ghost-CLI tests
  • GitHub Check: Unit tests (Node 22.18.0)
  • GitHub Check: Legacy tests (Node 22.18.0, sqlite3)
  • GitHub Check: Legacy tests (Node 22.18.0, mysql8)
  • GitHub Check: Acceptance tests (Node 22.18.0, mysql8)
  • GitHub Check: Acceptance tests (Node 22.18.0, sqlite3)
  • GitHub Check: Lint
🔇 Additional comments (10)
ghost/core/core/server/models/automated-email.js (6)

1-3: LGTM!

The imports are correct and follow Ghost's standard pattern for models with lexical content transformation.


8-12: LGTM!

The default status of 'inactive' is appropriate for new automated emails, ensuring they must be explicitly activated.


14-23: LGTM!

The parse method correctly transforms stored URLs from __GHOST_URL__ placeholders to absolute URLs when reading from the database. The guard for missing lexical content is appropriate.


5-6: Schema is properly defined.

The automated_emails table schema is defined in the migration file ghost/core/core/server/data/migrations/versions/6.10/2025-12-01-21-04-36-add-automated-emails-table.js, which correctly matches the model's tableName reference.


39-41: The model export is correct. Ghost's models index file uses glob-based dynamic loading (glob.sync('!(index).js', ...)) that automatically imports and merges all model files, so AutomatedEmail will be accessible as models.AutomatedEmail without requiring explicit registration in the index file.


25-36: Verification complete: lexicalLib exports are correctly structured.

The formatOnWrite method correctly transforms absolute URLs to __GHOST_URL__ placeholders for storage. Both lexicalLib.nodes (getter returning DEFAULT_NODES from @tryghost/kg-default-nodes) and lexicalLib.urlTransformMap (getter returning object with absoluteToRelative, relativeToAbsolute, and toTransformReady URL transformation methods) are properly exported and match the expected structure for the URL transformation utility.

ghost/core/test/unit/server/models/automated-email.test.js (4)

1-7: LGTM!

The test setup follows Ghost's standard pattern for model unit tests with proper initialization.


9-25: LGTM!

The defaults tests are comprehensive, verifying both the default value and that only expected defaults are present.


27-77: LGTM!

The parse tests comprehensively cover URL transformation, edge cases (null/undefined), and field preservation. The assertions properly verify both positive and negative conditions.


79-128: LGTM!

The formatOnWrite tests comprehensively mirror the parse tests, covering the inverse URL transformation and all edge cases. The symmetric test coverage ensures bidirectional transformation reliability.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@cmraible cmraible force-pushed the chris-ny-771-add-automated_email-bookshelf-model branch from 0a07b79 to cd97fb0 Compare November 20, 2025 05:03
@cmraible cmraible force-pushed the chris-ny-770-create-database-table-migration-to-add-automated_emails branch 3 times, most recently from 81d3458 to 0947e94 Compare December 1, 2025 22:21
@cmraible cmraible force-pushed the chris-ny-771-add-automated_email-bookshelf-model branch from cd97fb0 to 8abdaeb Compare December 1, 2025 22:23
@cmraible cmraible force-pushed the chris-ny-770-create-database-table-migration-to-add-automated_emails branch from af2f8a5 to f9be63c Compare December 2, 2025 00:46
@cmraible cmraible force-pushed the chris-ny-771-add-automated_email-bookshelf-model branch from 8abdaeb to d5a0316 Compare December 2, 2025 01:04
@cmraible cmraible changed the title Added automated_email bookshelf model Added automated_email bookshelf model Dec 2, 2025
@cmraible cmraible marked this pull request as ready for review December 2, 2025 01:22
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds a new AutomatedEmail bookshelf model to enable CRUD operations on the automated_emails table, which will store customizable welcome email content for new members with lexical field support.

  • Implements the AutomatedEmail model with URL transformation for lexical content fields
  • Includes comprehensive unit tests covering defaults, URL parsing, and formatting operations
  • Follows established patterns from similar models (snippet, post) for lexical field URL transformations

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
ghost/core/core/server/models/automated-email.js Implements the AutomatedEmail model with table mapping, default status, and URL transformation methods (parse/formatOnWrite) for lexical fields
ghost/core/test/unit/server/models/automated-email.test.js Adds comprehensive unit tests for defaults, parse() URL transformations, and formatOnWrite() URL transformations including edge cases (null/undefined)

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Contributor

@troyciesco troyciesco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cmraible this one LGTM once its blocking PR is merged 👍

@cmraible cmraible force-pushed the chris-ny-770-create-database-table-migration-to-add-automated_emails branch from 315909d to 913bba0 Compare December 2, 2025 20:09
@cmraible cmraible force-pushed the chris-ny-771-add-automated_email-bookshelf-model branch 2 times, most recently from a2f0f1a to 4c045e2 Compare December 2, 2025 20:23
Base automatically changed from chris-ny-770-create-database-table-migration-to-add-automated_emails to main December 2, 2025 21:21
@cmraible cmraible force-pushed the chris-ny-771-add-automated_email-bookshelf-model branch from 4c045e2 to 436338c Compare December 2, 2025 21:27
@cmraible cmraible merged commit 427fbb2 into main Dec 2, 2025
31 of 33 checks passed
@cmraible cmraible deleted the chris-ny-771-add-automated_email-bookshelf-model branch December 2, 2025 22:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants