[pull] main from TryGhost:main - #1426
Merged
Merged
Conversation
no ref Every settings dialog now renders as a controlled component, either from its route or from the component that opens it, so nothing is created or shown through NiceModal any more. This removes: - the `NiceModal.Provider` from the settings app - the NiceModal fallback paths from Shade's `SettingsModal` and the settings `PreviewModalContent` — `onClose` is now required on both (every consumer already passed it) - the `@ebay/nice-modal-react` dependency from admin, Shade and admin-x-framework (which had it listed but never imported it), plus its catalog entry and lockfile entries
no ref `SettingsModal.onOk` (Shade), `TopLevelGroup.onSave` and the Pintura editor's `handleSave` are all passed async handlers throughout settings, but were typed as returning `void`. Under the type-checked lint rules that apply outside the settings quarantine, every one of those call sites is a `no-misused-promises` error — 30-odd sites across the areas about to move out of `settings/app/`. The callers fire the handler and do not await it, so the honest type is `void | Promise<…>`, with the callers discarding the result explicitly (`void onOk()`). This lets the settings code move out of the quarantine without wrapping dozens of handlers in `() => void handle()` arrows.
no ref This is a test-only change that covers more code paths.
no ref First area to leave `settings/app/`: the general settings components now live beside their acceptance tests in `settings/general`. Files outside `settings/app` run the type-aware lint rules the rest of admin uses, so the move also clears that debt for these files: - async handlers passed to void callbacks are wrapped (`void …`) or extracted into named handlers - `searchKeywords` moves to `general/search-keywords.ts` so `general-settings.tsx` only exports components - `GHOST_BUILD_VERSION` is declared on `ImportMetaEnv` instead of being `any` - the invite error payload is narrowed instead of `as any` `withErrorBoundary` is registered as an `extraHOCs` entry for `react-refresh/only-export-components` — every settings section exports through it and the rule can't recognise it as a component otherwise (this was ~80 of the ~340 quarantine errors across all areas). Nothing changes for the remaining areas; the ESLint override for `src/settings/app/**` shrinks by attrition and deletes with the last chunk.
ref https://linear.app/ghost/issue/PLA-137 Groundwork for the full-repo Oxfmt cutover: `pnpm format` / `pnpm format:check` root scripts, so there is one spelling for running the formatter. No enforcement yet — the CI check and lint-staged wiring land with the cutover so there is no half-state where CI reports thousands of unformatted files.
…ver (#30078) ref https://linear.app/ghost/issue/PLA-137 Groundwork so the upcoming full-repo Oxfmt run can be a pure formatting commit. Formatting everything and then linting surfaced two classes of breakage; both are fixed here without any formatting churn.
no ref The comments pagination acceptance test now waits for the second thread request to be captured and for the final reply to render before checking the aggregate row count and completed pagination state.
closes https://linear.app/ghost/issue/NY-1532/animate-automation-canvas-recentering-when-deleting-the-final-step A corrective viewport clamp after deleting the final step should make the workflow movement clear instead of snapping. Ignore floating-point zoom noise from the programmatic pan so it cannot restart its own transition. Video shows the nodes from the end of the list getting deleted and the canvas recentering smoothly. note it also shows that deleting a node from the middle of the workflow will _not_ animate smoothly, it just immediately goes away like it did before. The code for implementing that will be a little more involved and handled in a separate PR. https://github.com/user-attachments/assets/5bc13334-ec5d-42e6-a939-b308edf42956
no ref - the pr cache was never used (cache-from was always main) so the prior config meant a heavy push for no gain. Removing the cache-to on PRs will hopefully reduce the rate limit exceeded errors we get from Github
no ref Renovate had no automergeStrategy set, so automerges used the platform default (squash). Squash merges compose the commit message from the PR title and full PR body, and Renovate PR bodies are huge (release-note tables, changelogs), producing giant commit messages on main. Renovate branches are a single concise commit, so rebasing preserves that clean message verbatim.
…0087) no ref The msttcorefonts installer downloads the font files from SourceForge at install time, and its mirror-redirect roulette can hang indefinitely. Without any timeout, a stuck fetch sat until the whole job timeout killed it, wasting a full CI slot on the koenig-lexical acceptance leg. Bound apt's network ops with Acquire retries/timeouts and wrap the install in a timeout-guarded retry loop (SIGKILL 5s after SIGTERM) so a stuck mirror is abandoned after 120s and re-rolled onto a different one. Track success across attempts and exit 1 if all three fail, so a missing font install can't let the step pass. timeout-minutes: 10 is a backstop above the worst-case retry path for any hang the inner timeout can't catch.
) closes https://linear.app/ghost/issue/NY-1530 ref ce08414 ref 1773748 This was written by Claude Opus 5 with the following prompt: > Commit `ce0841435c44a24bd444277cc587e3a18adc052d` adds a "last run created at" stat to the automation browse endpoint. Commit `1773748282248ee9af3cdca8670023e531ea67e6` built on top of that and added `total_run_count`. > > I want a new key, `in_progress_run_count`, which is a count of all the runs for that automation that have any `pending` steps. > > Use red/green TDD. Re-generate the snapshots with `UPDATE_SNAPSHOTS=1` and running the necessary tests. > > This should be a fairly straightforward change, except maybe the database query. A tiny amount of additional prompting (removing some comments and unnecessary test assertions) followed. Co-authored-by: Claude <noreply@anthropic.com>
Two behaviours the field picker already has were covered nowhere: choosing the second of two same-named fields from the keyboard, which depends on each cmdk item having its own identity, and searching against the names shown on the rows rather than the namespaced column values behind them. Both tests were verified to fail when the code that makes them pass is reverted, so they pin the behaviour ahead of the refactor that reshapes how targets reach the picker.
…cker The field picker inferred what kind of thing had been selected from which of two arrays it was found in, and recovered a composite field's name and part by running a regex back over a label the framework had already joined from them. Both are now given rather than derived: a CSV column carries its field name and part label alongside the joined form, and a new field-targets module builds a single list of targets each carrying its own source and whether its name is contested by another field. A FIELD_SOURCES record keyed by the source union turns adding a third source, such as Stripe, into a build failure until it has a heading, a badge decision, an accessible name and an icon. No behaviour change.
no ref Second area to leave `settings/app/`: membership components now live beside their acceptance tests in `settings/membership`. Moving them enables the type-aware lint rules the rest of admin uses, so that debt is cleared for these files: - async handlers passed to void callbacks are wrapped; fire-and-forget promises get `void` - `searchKeywords` → `membership/search-keywords.ts`; the welcome-email design payload mapping (`mapApiToDesignSettings`, `buildAutomatedEmailDesignPayload`, …) → `member-emails/design-payload.ts` (its test already had that name), so component files only export components - Koenig `EmailEditor` read and the lexical JSON parse are narrowed instead of flowing as `any`; the token-expiry catch uses `instanceof Error` - settings `PreviewModalContent.onOk` and `EmailDesignModal.onSave` widened to accept async handlers (same treatment as `SettingsModal.onOk` in #30076) — the portal and customize dialogs pass them
no ref `NODE_ENV` may be missing. Let's handle that. This change should have no user impact.
no ref - waits for the nested thread's exact `href` before clicking the replies metric - keeps the existing route and rendered-thread assertions unchanged
towards https://linear.app/ghost/issue/NY-1535 closes https://linear.app/ghost/issue/NY-1536 - Apply the configured `bulkEmail:mailgun:tag` to real automation email sends. - Preserve the existing `automation-email` classification and untagged fallback. - Leave automation analytics filtering unchanged so production can transition safely before requiring the configured tag. Automation sends call the lower-level Mailgun client directly and therefore bypass the provider that adds the configured site tag to newsletters. Adding the tag at the automation send boundary fixes that gap without prematurely excluding events from automation emails sent before this rollout.
no ref Third area to leave `settings/app/`: growth components now live beside their acceptance tests in `settings/growth`. Moving them enables the type-aware lint rules the rest of admin uses, so that debt is cleared for these files: - async handlers passed to void callbacks are wrapped; fire-and-forget promises get `void` - non-component exports move into their own modules so component files only export components: `searchKeywords` → `growth/search-keywords.ts`; `getOfferCadence/Duration/Discount` + `OfferType` → the existing `offers/offer-helpers.ts`; `validateDescriptionForm[Field]` → `recommendations/recommendation-validation.ts`
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
7 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
See Commits and Changes for more details.
Created by
pull[bot] (v2.0.0-alpha.4)
Can you help keep this open source service alive? 💖 Please sponsor : )