Skip to content

Changed route settings service API names to upload and download - #29364

Merged
vershwal merged 1 commit into
mainfrom
renameUploadDownload
Jul 15, 2026
Merged

Changed route settings service API names to upload and download#29364
vershwal merged 1 commit into
mainfrom
renameUploadDownload

Conversation

@vershwal

Copy link
Copy Markdown
Member

ref https://linear.app/ghost/issue/HKG-1830

  • the service methods now carry the names of the Admin API operations they back: get was ambiguous sitting next to store.get(), and setFromFilePath describes a transport detail rather than what the caller is doing
  • mechanical rename only — signatures and behavior are unchanged, and upload still takes the multer tmp-file path; moving the file read to the API boundary (so the service's only I/O is the configured store) ships separately on top of this

@vershwal
vershwal requested a review from Copilot July 15, 2026 18:19
@vershwal
vershwal force-pushed the renameUploadDownload branch from b332bff to 24579d9 Compare July 15, 2026 18:19
@coderabbitai

coderabbitai Bot commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: b5050c6d-badc-4f95-a5b8-2545ffc1e117

📥 Commits

Reviewing files that changed from the base of the PR and between 24579d9 and 0ec451e.

📒 Files selected for processing (4)
  • ghost/core/core/server/api/endpoints/settings.js
  • ghost/core/core/server/services/route-settings/dynamic-routing-service.js
  • ghost/core/core/server/services/route-settings/index.js
  • ghost/core/test/unit/server/services/route-settings/dynamic-routing-service.test.ts
🚧 Files skipped from review as they are similar to previous changes (3)
  • ghost/core/core/server/services/route-settings/index.js
  • ghost/core/core/server/api/endpoints/settings.js
  • ghost/core/test/unit/server/services/route-settings/dynamic-routing-service.test.ts

Walkthrough

Renames DynamicRoutingService methods from get and setFromFilePath to download and upload. Updates route-settings API delegation getters and settings endpoints to use the new names. Adjusts unit tests for retrieval, fallback, validation, persistence, corrupt store content, and frontend reload failure scenarios.

Possibly related PRs

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: renaming the route settings API methods to upload and download.
Description check ✅ Passed The description matches the changeset and correctly explains the mechanical rename and unchanged behavior.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch renameUploadDownload

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.

@nx-cloud

nx-cloud Bot commented Jul 15, 2026

Copy link
Copy Markdown

🤖 Nx Cloud AI Fix

Ensure the fix-ci command is configured to always run in your CI pipeline to get automatic fixes in future runs. For more information, please see https://nx.dev/ci/features/self-healing-ci


View your CI Pipeline Execution ↗ for commit 24579d9

Command Status Duration Result
nx run ghost:test:ci:integration ✅ Succeeded 1m 54s View ↗
nx run ghost:test:integration ✅ Succeeded 2m 51s View ↗
nx run ghost:test:legacy ✅ Succeeded 2m 59s View ↗
nx run ghost:test:e2e ✅ Succeeded 2m 32s View ↗
nx run ghost-monorepo:lint:boundaries ✅ Succeeded 19s View ↗
nx run-many -t test:unit -p ghost ✅ Succeeded 30s View ↗
nx run-many -t lint -p ghost,ghost-monorepo ✅ Succeeded 23s View ↗
nx run @tryghost/admin:build ✅ Succeeded 7s View ↗
nx run-many --target=build --projects=tag:publi... ✅ Succeeded <1s View ↗

💡 Verify your cache is correct by running tasks in a sandbox. Read docs ↗


☁️ Nx Cloud last updated this comment at 2026-07-15 18:39:29 UTC

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Renames the route settings service API methods to align with the Admin API operations they back (upload/download), updating the service, API endpoint usage, and unit tests accordingly.

Changes:

  • Renamed DynamicRoutingService.get()download() and setFromFilePath()upload().
  • Updated the settings API endpoint handlers to call routeSettings.api.upload() / routeSettings.api.download().
  • Updated unit tests to use the new method names.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
ghost/core/test/unit/server/services/route-settings/dynamic-routing-service.test.ts Updates unit tests to use download/upload method names.
ghost/core/core/server/services/route-settings/index.js Renames the exported api method accessors to upload/download (potential compatibility concern noted in review comment).
ghost/core/core/server/services/route-settings/dynamic-routing-service.js Renames service methods to download/upload while keeping behavior intact.
ghost/core/core/server/api/endpoints/settings.js Updates API endpoint wiring to call the renamed route-settings API methods.

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

Comment thread ghost/core/core/server/services/route-settings/index.js
ref https://linear.app/ghost/issue/HKG-1830

- the service methods now carry the names of the Admin API operations
  they back: `get` was ambiguous sitting next to `store.get()`, and
  `setFromFilePath` describes a transport detail rather than what the
  caller is doing
- mechanical rename only — signatures and behavior are unchanged, and
  upload still takes the multer tmp-file path; moving the file read to
  the API boundary (so the service's only I/O is the configured store)
  ships separately on top of this
@vershwal
vershwal force-pushed the renameUploadDownload branch from 24579d9 to 0ec451e Compare July 15, 2026 18:29
@vershwal
vershwal enabled auto-merge (squash) July 15, 2026 18:34
@codecov

codecov Bot commented Jul 15, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 74.04%. Comparing base (77790bf) to head (0ec451e).

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #29364      +/-   ##
==========================================
+ Coverage   74.01%   74.04%   +0.02%     
==========================================
  Files        1578     1578              
  Lines      137508   137508              
  Branches    16648    16652       +4     
==========================================
+ Hits       101772   101812      +40     
+ Misses      34691    34681      -10     
+ Partials     1045     1015      -30     
Flag Coverage Δ
e2e-tests 76.12% <100.00%> (+0.03%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@vershwal
vershwal merged commit a7c4b00 into main Jul 15, 2026
45 checks passed
@vershwal
vershwal deleted the renameUploadDownload branch July 15, 2026 18:42
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.

2 participants