Add company-data-copier command

[BenjaminLangenakenSF]

What

Adds a new silverfin company-data-copier CLI command that triggers the platform's Data Copier to copy a source company's data into a brand-new company in a destination (development) firm. This lets BSO developers reproduce a client's situation in a dev firm to debug templates against realistic data, without ever touching the production firm.

silverfin company-data-copier -c <source_company_id> -l <period_id_1> <period_id_2> -f <destination_firm_id>

• -c, --source-company-id — company to copy data from (in the source/client firm)
• -l, --source-ledger-ids — one or more period ids, space-separated (variadic, matching run-test's --handle)
• -f, --firm — destination firm whose token authenticates the request and where the copy is created (defaults to the configured default firm)

It copies data only (account values incl. adjustments, text properties, people/company drop, configuration) — not template code. The destination firm is taken from the URL/token; the source is identified purely by company + period ids in the body (no source-firm parameter). The platform enforces source-firm access rights on the authenticating user (see the access note below).

How it wires up

Follows the existing layering exactly:

• lib/api/sfApi.js → runCompanyDataCopier, reusing AxiosFactory.createInstance (so staging Basic-auth, access_token, and token refresh all work unchanged).
• index.js → copyCompanyData (input validation + reporting).
• bin/cli.js → the company-data-copier command.

✅ Route verified on live — it is the firm-scoped v4 route

An earlier revision of this command POSTed to the public, non-firm-scoped /api/public/v3/company_data_copier/run, on the basis that doc/bso_company_data_copier.md said the firm-scoped route had been renamed away (commit 24e2547bc9). Live probing shows that is not the case — the copier is served at the firm-scoped route:

Request on live.getsilverfin.com	Result
POST /api/public/v3/company_data_copier/run	404 page not found — route does not exist
POST /api/v4/f/:id/company_data_copier/run	401 invalid_token — route exists, only auth missing; returns a proper 422 business-validation once authenticated

So this PR now POSTs the relative company_data_copier/run, inheriting the firm axios instance's /api/v4/f/:id baseURL (and its token, staging Basic-auth and refresh interceptor) — the "revert this one line" the previous note anticipated.

@team-reconciliation — the doc's claim that /api/v4/f/:id/company_data_copier/run was renamed away in favour of a public-v3 route does not match the deployed live behaviour. Please update doc/bso_company_data_copier.md.

Two other alignments with the merged backend (unchanged from the previous revision):

• Copied-company name. The backend names the company Company_<source_id>_<hex> (e.g. Company_42_a3f1b9d4), not SF_COPY_<id>_<timestamp>. Success message + README reflect this.
• Fire-and-forget, honestly. 202 {"status":"enqueued"} means enqueued, not copied — the copy runs in a Sidekiq job and there is no status endpoint. Messaging says the copy was requested, that it runs async, to verify in the destination firm after a few minutes, and that a company name ending in _FAILED means the (non-retried) copy failed.

⚠️ Testing with real client data requires a support user

The feature's purpose is reproducing client data in a dev firm, but doing so runs into an access boundary worth flagging explicitly:

The command authenticates as the destination-firm user, and the backend resolves the source company in that user's context. Reading a company that lives in a different (client) firm therefore requires the authenticating user to be either:

• a Silverfin support user (silverfin_support?), which can read any firm's companies, or
• a member of both the source and destination firms.

A normal BSO-developer account cannot reach client firms — the OAuth authorize page only lists the internal dev/demo/testing firms the user belongs to — so attempting a client source yields 422 {"error":"source_company_id <id> does not exist"} (the backend reports cross-firm invisibility as "does not exist"). This is almost certainly intentional given the data-protection implications of copying production client data into a dev firm.

Open question for the team: how are BSO developers meant to authenticate for real client-data copies? Options: a scoped support credential for this workflow, support-initiated copies, or a consent/approval flow. Note that copies within firms the developer already belongs to (e.g. functional-testing → demo) work today and are sufficient to validate the command itself end-to-end (404 → 401 → 422 validation → 202 enqueued).

Backend prerequisites / gotchas (now in the README)

These cause 422s or silent skips that would otherwise confuse users:

• Destination firm must be a demo firm (trial/customer/junk/churn → 422).
• Source firm must differ from destination firm (422).
• All source_ledger_ids must belong to source_company_id (422).
• Templates are matched to the destination firm by handle (reconciliations) / Dutch name name_nl (account detail templates). If the template doesn't already exist in the destination firm, its data is silently skipped — so the template you're debugging must already exist there.
• Caller must be a Silverfin support user or have access to the source firm (see access note above).
• One copy per destination firm at a time (Redis mutex, 5-min TTL). A second run fired at the same destination while one is in flight silently does nothing — don't fire the command repeatedly.

Also included

• Removed a misleading suffix from the shared 422 error handler (apiUtils.js) — a 422 isn't specifically a permissions issue, so "You don't have the rights to update the previous parameters" was confusing (it showed on a Data Copier "source_company_id does not exist" error). Now only the real API error is surfaced. Applies to all commands.
• README section + CHANGELOG entry, version bump to 1.57.0.

Testing

• sfApi HTTP layer (axios-mock-adapter) asserts the request hits the firm-scoped company_data_copier/run route (relative to the firm /api/v4/f/:id baseURL).
• E2E command test (tests/bin/cli/) asserts the success message reports requested/enqueued and the follow-up guidance flags the async nature + _FAILED signal.
• Commander wiring in tests/bin/cli.test.js, plus TESTS.md catalogue entries.
• Full suite green, lint clean.

🤖 Generated with Claude Code

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

Walkthrough

Adds the company-data-copier CLI command and supporting toolkit/API functions to enqueue source company data copies into a destination firm. Includes input validation, response handling, tests, documentation, changelog updates, version bump, and revised 422 error logging.

Changes

Company Data Copier Feature

Layer / File(s)	Summary
API integration lib/api/sfApi.js, lib/utils/apiUtils.js, tests/lib/api/sfApi.test.js	Adds the firm-scoped company_data_copier/run request helper, exports it, tests success and 422 handling, and logs serialized 422 response data.
Toolkit implementation index.js, tests/bin/cli/company-data-copier.test.js	Adds input validation, API invocation, result logging, public export, and tests for success, missing inputs, and missing response data.
CLI wiring and validation bin/cli.js, tests/bin/cli.test.js	Adds the company-data-copier subcommand with required options, numeric validation, failure exit handling, and help/exit-code tests.
Documentation and release metadata README.md, CHANGELOG.md, package.json, tests/TESTS.md	Documents command usage and prerequisites, adds the 1.57.0 changelog entry, updates the package version, and records related test coverage.

Estimated code review effort: 3 (Moderate) | ~25 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title is concise and clearly names the primary change: adding the company-data-copier command.
Description check	✅ Passed	The description is detailed and covers the change, wiring, and testing, though it does not follow the template sections exactly.
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 💡 1

📝 Generate docstrings 💡

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch add-company-data-copier-command

---

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

🧹 Nitpick comments (2)

tests/bin/cli/company-data-copier.test.js (2)

77-84: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Test only covers undefined response, not a "truthy response without .data" case.

The guard in copyCompanyData is !response || !response.data; only the !response branch is exercised here. Consider adding a case like SF.runCompanyDataCopier.mockResolvedValue({}) to also cover the "response exists but has no data" path.

🤖 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 `@tests/bin/cli/company-data-copier.test.js` around lines 77 - 84, The current
test for copyCompanyData only covers the undefined response path, but not the
case where SF.runCompanyDataCopier returns an object without data. Update the
company-data-copier test suite to add a separate case that mock-resolves a
truthy response with no data (for example via the copyCompanyData flow) and
asserts the same error/false behavior, so both branches of the !response ||
!response.data guard are exercised.

---

21-44: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Unused e2e-style scaffolding in a pure unit test suite.

tempDir/chdir/process.exit mocking (lines 24-30, 41-43) are never exercised — copyCompanyData doesn't touch the filesystem or call process.exit. This looks copy-pasted from an e2e CLI test file and just adds noise.

♻️ Suggested simplification

-const fsPromises = require("fs").promises;
-const path = require("path");
-const os = require("os");
-
 jest.mock("consola");
 jest.mock("../../../lib/api/sfApi");

 const SF = require("../../../lib/api/sfApi");
 const consola = require("consola");
 const toolkit = require("../../../index");

 describe("company-data-copier", () => {
-  let tempDir;
-  let originalCwd;
-  let originalExit;
-
   const destinationFirmId = 13692;
   const sourceCompanyId = 1224550;
   const sourceLedgerIds = [33417839, 32116688];

   beforeEach(async () => {
     jest.clearAllMocks();
-
-    tempDir = await fsPromises.mkdtemp(path.join(os.tmpdir(), "sf-cli-test-"));
-
-    originalCwd = process.cwd();
-    process.chdir(tempDir);
-
-    originalExit = process.exit;
-    process.exit = jest.fn();

     consola.success = jest.fn();
     consola.error = jest.fn();
     consola.info = jest.fn();
     consola.log = jest.fn();
     consola.warn = jest.fn();
     consola.debug = jest.fn();
   });
-
-  afterEach(async () => {
-    process.chdir(originalCwd);
-    process.exit = originalExit;
-    await fsPromises.rm(tempDir, { recursive: true, force: true });
-  });

🤖 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 `@tests/bin/cli/company-data-copier.test.js` around lines 21 - 44, The test
setup in company-data-copier.test.js includes unused e2e-style scaffolding that
`copyCompanyData` never exercises. Remove the unnecessary `tempDir` creation,
`process.chdir`/`originalCwd` handling, and `process.exit` mocking from
`beforeEach`/`afterEach`, and keep only the mocks actually needed for the unit
tests such as the `consola` methods and `jest.clearAllMocks()`.

🤖 Prompt for all review comments with 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.

Nitpick comments:
In `@tests/bin/cli/company-data-copier.test.js`:
- Around line 77-84: The current test for copyCompanyData only covers the
undefined response path, but not the case where SF.runCompanyDataCopier returns
an object without data. Update the company-data-copier test suite to add a
separate case that mock-resolves a truthy response with no data (for example via
the copyCompanyData flow) and asserts the same error/false behavior, so both
branches of the !response || !response.data guard are exercised.
- Around line 21-44: The test setup in company-data-copier.test.js includes
unused e2e-style scaffolding that `copyCompanyData` never exercises. Remove the
unnecessary `tempDir` creation, `process.chdir`/`originalCwd` handling, and
`process.exit` mocking from `beforeEach`/`afterEach`, and keep only the mocks
actually needed for the unit tests such as the `consola` methods and
`jest.clearAllMocks()`.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: c6576f3c-3c1f-4198-8542-fb631ebcb724

📥 Commits

Reviewing files that changed from the base of the PR and between 258e6b7 and 2f72045.

⛔ Files ignored due to path filters (1)

• package-lock.json is excluded by !**/package-lock.json

📒 Files selected for processing (11)

• CHANGELOG.md
• README.md
• bin/cli.js
• index.js
• lib/api/sfApi.js
• lib/utils/apiUtils.js
• package.json
• tests/TESTS.md
• tests/bin/cli.test.js
• tests/bin/cli/company-data-copier.test.js
• tests/lib/api/sfApi.test.js

[michieldegezelle]

💡 Suggestion: This catalogue entry still describes the old public-v3 absolute URL, but `sfApi.test.js` and the implementation now use the firm-scoped relative `company_data_copier/run` path — please align the test name and description with the current route.

[BenjaminLangenakenSF]

Implemented with Align TESTS.md catalogue entry with the firm-scoped Data Copier route

[michieldegezelle]

🟠 Major: `copyCompanyData` returns `false` when the API yields no response data (404/400 path via `responseErrorHandler`), but this action ignores the return value so the process still exits 0 — scripts/CI will treat a failed copy request as success.

const result = await toolkit.copyCompanyData(options.firm, sourceCompanyId, sourceLedgerIds);
if (result === false) {
  process.exit(1);
}

[BenjaminLangenakenSF]

Implemented with Exit non-zero when the Data Copier request fails

[coderabbitai]

🧹 Nitpick comments (1)

tests/bin/cli.test.js (1)

129-142: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy lift

Cover the API-failure exit path.

These tests only exercise Commander validation failures; none makes toolkit.copyCompanyData return false after the request fails. Add a regression test asserting that this path exits with status 1, so the new failure branch in bin/cli.js is actually protected.

🤖 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 `@tests/bin/cli.test.js` around lines 129 - 142, Add a regression test in the
“silverfin company-data-copier exit codes” suite that makes the API request used
by toolkit.copyCompanyData fail and return false, then assert the CLI exits with
status 1. Keep the existing Commander validation tests unchanged and target the
new failure branch in bin/cli.js.

🤖 Prompt for all review comments with 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.

Nitpick comments:
In `@tests/bin/cli.test.js`:
- Around line 129-142: Add a regression test in the “silverfin
company-data-copier exit codes” suite that makes the API request used by
toolkit.copyCompanyData fail and return false, then assert the CLI exits with
status 1. Keep the existing Commander validation tests unchanged and target the
new failure branch in bin/cli.js.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 51b09c08-dac0-43d3-924c-374e0eea4099

📥 Commits

Reviewing files that changed from the base of the PR and between 5225462 and ada0a53.

📒 Files selected for processing (3)

• bin/cli.js
• tests/TESTS.md
• tests/bin/cli.test.js

🚧 Files skipped from review as they are similar to previous changes (1)

• tests/TESTS.md