Migrations for domain warmup feature #25330
Conversation
github-actions
bot
added
the
migration
|
It looks like this PR contains a migration 👀 General requirements
Schema changes
Data changes
|
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughThis PR adds two migrations and corresponding schema changes: a nullable integer column Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Pre-merge checks and finishing touches✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (7)
🚧 Files skipped from review as they are similar to previous changes (6)
🧰 Additional context used🧠 Learnings (1)📚 Learning: 2025-10-09T17:25:12.439ZApplied to files:
⏰ 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). (8)
🔇 Additional comments (1)
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. Comment |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
ghost/core/core/server/data/migrations/versions/6.7/2025-11-03-15-17-05-add-csd-email-count.js(1 hunks)ghost/core/core/server/data/migrations/versions/6.7/2025-11-03-15-18-04-add-email-batch-fallback-domain.js(1 hunks)ghost/core/core/server/data/schema/schema.js(2 hunks)ghost/core/test/unit/server/data/schema/integrity.test.js(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
ghost/core/core/server/**
📄 CodeRabbit inference engine (AGENTS.md)
Backend core logic should reside under ghost/core/core/server/
Files:
ghost/core/core/server/data/schema/schema.jsghost/core/core/server/data/migrations/versions/6.7/2025-11-03-15-17-05-add-csd-email-count.jsghost/core/core/server/data/migrations/versions/6.7/2025-11-03-15-18-04-add-email-batch-fallback-domain.js
ghost/core/core/server/data/schema/**
📄 CodeRabbit inference engine (AGENTS.md)
Database schema changes must be placed under ghost/core/core/server/data/schema/
Files:
ghost/core/core/server/data/schema/schema.js
🧠 Learnings (1)
📚 Learning: 2025-10-09T17:25:12.439Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-10-09T17:25:12.439Z
Learning: Applies to ghost/core/core/server/data/schema/** : Database schema changes must be placed under ghost/core/core/server/data/schema/
Applied to files:
ghost/core/test/unit/server/data/schema/integrity.test.jsghost/core/core/server/data/schema/schema.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). (6)
- GitHub Check: Ghost-CLI tests
- GitHub Check: Legacy tests (Node 22.13.1, mysql8)
- GitHub Check: Legacy tests (Node 22.13.1, sqlite3)
- GitHub Check: Lint
- GitHub Check: Unit tests (Node 22.13.1)
- GitHub Check: Build & Push
🔇 Additional comments (3)
ghost/core/core/server/data/migrations/versions/6.7/2025-11-03-15-18-04-add-email-batch-fallback-domain.js (1)
1-7: LGTM! Migration correctly adds fallback_sending_domain column.The migration properly uses the
createAddColumnMigrationutility and the column definition matches the schema definition inschema.js(line 870).ghost/core/test/unit/server/data/schema/integrity.test.js (1)
38-38: LGTM! Schema hash correctly updated.The updated MD5 hash reflects the schema changes (addition of
csd_email_countandfallback_sending_domaincolumns).ghost/core/core/server/data/schema/schema.js (1)
870-870: LGTM! Schema definition matches migration.The
fallback_sending_domaincolumn definition correctly matches the migration inghost/core/core/server/data/migrations/versions/6.7/2025-11-03-15-18-04-add-email-batch-fallback-domain.js.
ghost/core/core/server/data/migrations/versions/6.7/2025-11-03-15-17-05-add-csd-email-count.js
Show resolved
Hide resolved
sam-lord
force-pushed
the
domain-warmup-migrations
branch
from
14a793a to
a321b87
Compare
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
I think the acceptance tests need some care. Other than that, I think this migration is good. The naming of the new columns makes sense to me as well. I also agree with omitting the defaultTo: 0, so we have another indicator if the feature is used at all. I would expect that - when we turn the feature on and the user has a custom sending domain, but not send anything yet, we'd set it to 0?
sam-lord
force-pushed
the
domain-warmup-migrations
branch
from
f27c617 to
8732a18
Compare
| posts: [{ | ||
| title: 'My Newsletter', | ||
| html: `<p>External link <a href="https://example.com/a">https://example.com/a</a>; Internal link <a href=${siteUrl.href}/about">${siteUrl.href}/about</a>;Ghost homepage <a href="https://ghost.org">https://ghost.org</a></p>` | ||
| html: `<p>External link <a href="https://example.com/a">https://example.com/a</a>; Internal link <a href="${siteUrl.href}/about">${siteUrl.href}/about</a>;Ghost homepage <a href="https://ghost.org">https://ghost.org</a></p>` |
There was a problem hiding this comment.
I don't understand why I'm having to fix this 🤔
There was a problem hiding this comment.
That is strange indeed. Maybe something to flag up in our development channel?
closes [GVA-583](https://linear.app/ghost/issue/GVA-583/create-migration-in-ghost-to-add-csd-member-count-to-emails) We need to know how many emails were sent using the CSD from the previous email in order to ramp up the sending from the CSD. See [spike PR](#25303) for our test implementation of domain warmup using this field.
closes [GVA-587](https://linear.app/ghost/issue/GVA-587/create-a-migration-for-the-email-batches-table-to-say-whether-its-sent) We need to know whether an email batch should be sent from the fallback domain or the primary configured domain. See [spike PR](#25303) for our test implementation.
sam-lord
force-pushed
the
domain-warmup-migrations
branch
from
330a7ed to
ecb9bec
Compare
closes GVA-583
closes GVA-587
These migrations are both required for the feature. I ran an experimental spike to see what data we'd need to get domain warmup working, and these two came out:
Note
Adds DB migrations and schema/model/test updates for
emails.csd_email_countandemail_batches.fallback_sending_domain.emails.csd_email_count(nullable integer, unsigned) viaversions/6.7/2025-11-03-15-17-05-add-csd-email-count.js.email_batches.fallback_sending_domain(boolean, defaultfalse) viaversions/6.7/2025-11-03-15-18-04-add-email-batch-fallback-domain.js.core/server/data/schema/schema.jsto include both columns.core/server/models/email-batch.jsdefaults to includefallback_sending_domain: false.test/utils/fixtures/data-generator.jsto populatefallback_sending_domainand reflect new schema.test/unit/server/data/schema/integrity.test.js.csd_email_countand changedcontent-lengths.test/e2e-server/click-tracking.test.js(adds missing quotes).Written by Cursor Bugbot for commit ecb9bec. This will update automatically on new commits. Configure here.