Add commerce troubleshoot deploy command - #3
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a new Oclif command to deploy/update the Commerce Troubleshoot Console hosted page by delegating to @coveops/commerce-troubleshoot-deployer, with README usage examples and integration tests to validate flag/config mapping.
Changes:
- Introduce
org:commerce:troubleshoot:deploycommand that builds a deploy request from flags + Coveo config and prints deploy results/diagnostics. - Add integration tests covering managed vs provided key strategies and flag/config fallbacks.
- Add deployer dependency and document common deploy scenarios in
README.md.
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
src/commands/org/commerce/troubleshoot/deploy.ts |
New command implementation: config/flag resolution, key strategy selection, deployer invocation, result logging. |
test/commands/org/commerce/troubleshoot/deploy.test.ts |
New tests asserting request mapping and output for managed/provided strategies. |
package.json |
Adds @coveops/commerce-troubleshoot-deployer dependency. |
package-lock.json |
Locks the new deployer dependency. |
README.md |
Adds “Commerce Troubleshoot Console Deploy Examples” section with sample invocations. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| commerceTroubleshootDeployTestHooks.setReadConfiguration(() => ({ | ||
| ...DefaultConfig, | ||
| accessToken: 'cfg-access-token', | ||
| environment: 'prod', | ||
| organization: 'cfg-org', | ||
| region: 'us', | ||
| }) as never); |
There was a problem hiding this comment.
The test config stub is cast with as never, which defeats type checking and can mask real mismatches between the mocked configuration and the Configuration shape. Prefer returning a properly typed Configuration (e.g., as Configuration / satisfies Configuration) or avoid casting entirely if DefaultConfig already matches the type.
| commerceTroubleshootDeployTestHooks.setReadConfiguration(() => ({ | ||
| ...DefaultConfig, | ||
| accessToken: 'ignored-access-token', | ||
| environment: 'prod', | ||
| organization: 'ignored-org', | ||
| region: 'us', | ||
| }) as never); |
There was a problem hiding this comment.
Same issue here: casting the mocked configuration object to never removes useful compile-time guarantees in the test. Return a Configuration-typed object (or use satisfies Configuration) so the test fails if the command starts reading new config fields or the config shape changes.
| target: { | ||
| hostedPageName: flags.pageName, | ||
| organizationId, |
There was a problem hiding this comment.
--page-name is required, but the value is passed through without trimming/normalization. This allows whitespace-only values (or accidental leading/trailing spaces) to be sent to the deployer. Consider running flags.pageName through readString(...) and calling this.error(...) if it resolves to undefined, similar to organizationId/accessToken.
|
@copilot open a new pull request to apply changes based on the comments in this thread |
|
@jfallaire I've opened a new pull request, #4, to work on those changes. Once the pull request is ready, I'll request review from you. |
…ion (#4) * Initial plan * Fix as never casts in tests and normalize pageName flag Co-authored-by: jfallaire <7849359+jfallaire@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: jfallaire <7849359+jfallaire@users.noreply.github.com>
Summary
org:commerce:troubleshoot:deploycommand that wires into@coveops/commerce-troubleshoot-deployer, validates opts, and logs deploy resultsREADME.mdTesting