ghost/core/test/unit/server/models/post.test.js (1)

188-215: ⚡ Quick win

Add a companion test for the “already stable” order case.

Please add a test where order already contains id (for example published_at desc, id desc) with stableOrder: true, and assert no duplicate id ordering is appended. This protects the hasStableOrderField branch against regressions.

Suggested test shape

+        it('does not append duplicate stable order field when id is already present', function () {
+            const queries = [];
+            tracker.install();
+
+            tracker.on('query', (query) => {
+                queries.push(query);
+                query.response([]);
+            });
+
+            return models.Post.findPage({
+                filter: 'published_at:>\'2015-07-20\'',
+                order: 'published_at desc, id desc',
+                limit: 1,
+                skipPagination: true,
+                stableOrder: true
+            }).then(() => {
+                assert.equal(queries.length, 1);
+                assert.equal(
+                    queries[0].sql,
+                    'select `posts`.* from `posts` where (`posts`.`published_at` > ? and (`posts`.`type` = ? and `posts`.`status` = ?)) order by `posts`.`published_at` DESC, `posts`.`id` DESC limit ?'
+                );
+            });
+        });

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@ghost/core/test/unit/server/models/post.test.js` around lines 188 - 215, Add
a new unit test next to the existing "adds a stable order field..." test that
calls models.Post.findPage with stableOrder: true but with an order string that
already includes the id field (e.g. order: 'published_at desc, id desc');
install the tracker and capture the generated SQL and bindings, then assert that
the generated query does not append a duplicate `posts.id` ordering (SQL should
contain only one `posts.id` DESC and the bindings remain correct) and that
result.meta is still {}—this will exercise the hasStableOrderField branch in the
Post.findPage logic.

ghost/core/test/unit/server/services/posts/posts-exporter.test.js (2)

385-443: 💤 Low value

Default- and invalid-limit fallback coverage looks good.

Both tests correctly assert the 15-row default and the 'invalid' → default fallback. Minor optional cleanup: the three new pagination tests duplicate the Array.from({length: N}, ...) post-factory and the models.Post = { findPage: ... } mock; extracting a small buildPosts(n) / mockPaginatedPostModel(posts) helper inside this describe would shrink ~60 lines and make future additions cheaper. Not blocking.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@ghost/core/test/unit/server/services/posts/posts-exporter.test.js` around
lines 385 - 443, Extract the duplicated post factory and paginated model mock
into two helpers (e.g., buildPosts(count) that returns the Array.from(...) posts
using the existing post and email createModel shape, and
mockPaginatedPostModel(posts) that assigns models.Post = { findPage: async
(options) => { ... } } using the same pagination logic) and replace the repeated
blocks in the tests that call buildPosts(20) and mockPaginatedPostModel(posts)
before invoking exporter.export and collectStream; keep the helpers scoped
inside the describe so tests still reference post, exporter.export and
collectStream unchanged.

---

326-383: 💤 Low value

Snapshot test only exercises the “stable order requested” path.

The mock unconditionally appends id desc whenever options.stableOrder is set, and the exporter always passes stableOrder: true, so hasStableOrder is true on every call and the deliberately-swapped page-2 branch (Lines 351‑358) is never executed. The test still meaningfully snapshots the requested orders and confirms no duplicates/missing IDs, but it does not actually demonstrate that without stableOrder the export would be unstable. Consider adding a sibling case that calls into findPage without stableOrder (or removes the id desc injection) so the unstable branch is actually exercised — that would document why the option is required.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@ghost/core/test/unit/server/services/posts/posts-exporter.test.js` around
lines 326 - 383, The mock for models.Post.findPage always injects "id desc" when
options.stableOrder is true, and exporter.export currently always passes
stableOrder: true so the swapped-page (unstable) branch is never exercised; add
a sibling assertion that exercises the unstable path by calling the exporter (or
invoking findPage) without stableOrder: e.g. run a second collectStream(await
exporter.export(...)) call or temporarily override models.Post.findPage to
accept/see stableOrder: false so the hasStableOrder check is false and the
page-2 swap branch executes, then snapshot the requestedOrders/exportedIds for
that run to prove instability when stableOrder is not requested.

ghost/core/core/server/services/posts/posts-exporter.js (2)

19-35: 💤 Low value

parseExportLimit silently rewrites 0, negatives, and partially-numeric strings.

A few edge cases worth being aware of (not necessarily fixing):

• parseExportLimit(0) → 1 and parseExportLimit(-5) → 1 because of Math.max(parsed, 1). A caller passing 0 thinking it means “unbounded” will silently get a single row.
• Number.parseInt('15abc', 10) returns 15, so partially-numeric strings are accepted as their leading-numeric prefix rather than falling back to the default.

If the API contract is "be lenient, never error", the current behaviour is fine and consistent with the new tests. If you'd rather surface bad input, validating with Number.isFinite(Number(limit)) (or letting the input validator upstream reject it) would be more explicit. Flagging as optional.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@ghost/core/core/server/services/posts/posts-exporter.js` around lines 19 -
35, parseExportLimit currently accepts partially-numeric strings and uses
parseInt plus Math.max(parsed, 1), which silently coalesces inputs like "15abc",
0, and negatives; change parseExportLimit to validate strictly with
Number.isFinite(Number(limit)) and convert using Number(limit) (instead of
Number.parseInt) so "15abc" is rejected, then decide on the clamping behavior:
keep Math.max(parsed, 1) to preserve the current minimum-of-1 behavior or switch
to Math.max(parsed, 0) if you want to allow 0 as a valid value; reference
function parseExportLimit and DEFAULT_EXPORT_LIMIT when making this change.

---

104-137: 💤 Low value

One extra empty-page fetch when total rows are an exact multiple of pageLimit.

When requestedLimit === null (i.e. limit: 'all') and the underlying dataset size is an exact multiple of pageLimit, the loop won't break on the last non-empty page (since posts.data.length < pageLimit is false), so it issues one additional findPage call that returns an empty array before exiting via the data.length === 0 check on Line 119. Not a correctness bug, just one wasted DB roundtrip on the boundary. If you want to avoid it, you can track the previous page size or break when mapped.length is exactly the requested remainder. Otherwise, leave as-is — the cost is negligible and the code reads cleanly.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@ghost/core/core/server/services/posts/posts-exporter.js` around lines 104 -
137, The loop can do one extra DB fetch when total rows == exact multiple of
pageLimit; fix by using the pagination metadata returned by findPage to detect
the last page and avoid the extra fetch: in `#streamPosts`, after receiving posts
from this.#models.Post.findPage, check posts.meta?.pagination?.pages (or
equivalent meta.pages) and if the current page >= that total pages, break the
while loop instead of relying solely on posts.data.length < pageLimit; this uses
the existing posts result (from `#streamPosts` and posts.data) to stop iterating
without an extra roundtrip.