-
Notifications
You must be signed in to change notification settings - Fork 970
fix(core): auto-publish scheduled content and fix SQLite datetime comparison #1216
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| --- | ||
| "emdash": patch | ||
| --- | ||
|
|
||
| Fix scheduled posts never becoming published. Two independent bugs: | ||
|
|
||
| 1. **No auto-publish mechanism** -- `ContentRepository.findReadyToPublish()` existed but was never called outside tests. Added `publishScheduledContent()` that runs on every cron tick (Node scheduler or Cloudflare piggyback), iterates all collections, and publishes items whose `scheduled_at` has passed via the standard `publish()` path. Also wired `runtime.tickCron()` into middleware so the piggyback scheduler actually fires on Cloudflare Workers. | ||
|
|
||
| 2. **SQLite format mismatch (fixes #917)** -- `scheduled_at` is stored as ISO 8601 with `T` and `Z` (e.g. `2026-05-05T01:41:59.000Z`) but SQLite's `datetime('now')` returns `YYYY-MM-DD HH:MM:SS`. Lexicographic comparison sees `T` (0x54) > space (0x20), so `scheduled_at <= datetime('now')` was always false. Fixed by wrapping both sides in `datetime()` on SQLite in `loader.ts`, `content.ts`, and `snapshot.ts`. |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -893,15 +893,24 @@ export class ContentRepository { | |||||||||||||||||||
| * Returns all content where scheduled_at <= now, regardless of status. | ||||||||||||||||||||
| * This covers both draft-scheduled posts (status='scheduled') and | ||||||||||||||||||||
| * published posts with scheduled draft changes (status='published'). | ||||||||||||||||||||
| * | ||||||||||||||||||||
| * Uses datetime() on both sides for SQLite to normalize the ISO 8601 | ||||||||||||||||||||
| * "T"/"Z" format stored in scheduled_at against datetime('now')'s | ||||||||||||||||||||
| * "YYYY-MM-DD HH:MM:SS" format. On Postgres, casts to timestamptz. | ||||||||||||||||||||
| */ | ||||||||||||||||||||
| async findReadyToPublish(type: string): Promise<ContentItem[]> { | ||||||||||||||||||||
| const tableName = getTableName(type); | ||||||||||||||||||||
| const now = new Date().toISOString(); | ||||||||||||||||||||
|
|
||||||||||||||||||||
| const isPostgresDialect = this.db.getExecutor().adapter.constructor.name === "PostgresAdapter"; | ||||||||||||||||||||
| const scheduledAtExpr = isPostgresDialect | ||||||||||||||||||||
| ? sql`scheduled_at::timestamptz` | ||||||||||||||||||||
| : sql`datetime(scheduled_at)`; | ||||||||||||||||||||
| const nowExpr = isPostgresDialect ? sql`CURRENT_TIMESTAMP` : sql`datetime('now')`; | ||||||||||||||||||||
|
|
||||||||||||||||||||
| const result = await sql<Record<string, unknown>>` | ||||||||||||||||||||
| SELECT * FROM ${sql.ref(tableName)} | ||||||||||||||||||||
| WHERE scheduled_at IS NOT NULL | ||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [suggestion]
Suggested change
|
||||||||||||||||||||
| AND scheduled_at <= ${now} | ||||||||||||||||||||
| AND ${scheduledAtExpr} <= ${nowExpr} | ||||||||||||||||||||
| AND deleted_at IS NULL | ||||||||||||||||||||
| ORDER BY scheduled_at ASC | ||||||||||||||||||||
| `.execute(this.db); | ||||||||||||||||||||
|
|
||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -105,7 +105,7 @@ function isValidMetadataContribution(c: unknown): c is PageMetadataContribution | |||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| import { after } from "./after.js"; | ||||||||||||||||||||||||||
| import { loadBundleFromR2 } from "./api/handlers/marketplace.js"; | ||||||||||||||||||||||||||
| import { runSystemCleanup } from "./cleanup.js"; | ||||||||||||||||||||||||||
| import { publishScheduledContent, runSystemCleanup } from "./cleanup.js"; | ||||||||||||||||||||||||||
| import { | ||||||||||||||||||||||||||
| DEFAULT_COMMENT_MODERATOR_PLUGIN_ID, | ||||||||||||||||||||||||||
| defaultCommentModerate, | ||||||||||||||||||||||||||
|
|
@@ -1165,8 +1165,9 @@ export class EmDashRuntime { | |||||||||||||||||||||||||
| cronScheduler = new NodeCronScheduler(cronExecutor); | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| // Register system cleanup to run alongside each scheduler tick. | ||||||||||||||||||||||||||
| // Pass storage so cleanupPendingUploads can delete orphaned files. | ||||||||||||||||||||||||||
| // Register system cleanup and scheduled publishing to run | ||||||||||||||||||||||||||
| // alongside each scheduler tick. Both are independent and | ||||||||||||||||||||||||||
| // non-fatal -- failures are logged internally. | ||||||||||||||||||||||||||
| cronScheduler.setSystemCleanup(async () => { | ||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||
| await runSystemCleanup(db, storage ?? undefined); | ||||||||||||||||||||||||||
|
|
@@ -1175,6 +1176,14 @@ export class EmDashRuntime { | |||||||||||||||||||||||||
| // by runSystemCleanup. This catches unexpected errors. | ||||||||||||||||||||||||||
| console.error("[cleanup] System cleanup failed:", error); | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||
| await publishScheduledContent(db); | ||||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [needs fixing] On Cloudflare Workers,
The PR description itself notes that the piggyback scheduler fires promises without
Suggested change
|
||||||||||||||||||||||||||
| } catch (error) { | ||||||||||||||||||||||||||
| // Non-fatal -- individual publish failures are already logged | ||||||||||||||||||||||||||
| // by publishScheduledContent. This catches unexpected errors. | ||||||||||||||||||||||||||
| console.error("[scheduled] Scheduled content publishing failed:", error); | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| // Add cron reschedule callback (merges with existing factory options) | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[suggestion] This duplicates the dialect-detection logic that already lives in
dialect-helpers.ts. Using the sharedisPostgres(this.db)(orisSqlite(this.db)) helper is more maintainable and consistent withloader.ts,snapshot.ts, and the rest of the codebase.