Skip to content

[SAFE-EMAIL-TEST-REDUX-1] route testRunMetadata to action testRun handlers#1674

Open
ogp-weeloong wants to merge 1 commit into
chore/formsg-test-step-extensionfrom
feat/ses/safe-email-test/test-action-metadata
Open

[SAFE-EMAIL-TEST-REDUX-1] route testRunMetadata to action testRun handlers#1674
ogp-weeloong wants to merge 1 commit into
chore/formsg-test-step-extensionfrom
feat/ses/safe-email-test/test-action-metadata

Conversation

@ogp-weeloong

@ogp-weeloong ogp-weeloong commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

Problem

We always send test email to the pipe owner. However, they may have valid use case to send a test email to the configured recipient (e.g. people with multiple emails). We should support this.

Approach

We will reuse FormSG's approach to pass test run metadata to the backend.

  1. Postman will pass a new useConfiguredEmails test step metadata to backend. This defaults to false.
  2. If useConfiguredEmails === true, the test run handler will send test email To/Cc configured in the pipe. Otherwise it always sends to the pipe owner.

This PR

  • Wires up testRunMetadata to actions (since it's only used for triggers now)

Tests

Regression tests only for now; next PR will update postman to use this.

  • Check that test step with other app still works.
  • Check that formsg test step with mock and last submission both work.
  • Check that published pipes still work

ogp-weeloong commented Jun 4, 2026

Copy link
Copy Markdown
Contributor Author

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more


How to use the Graphite Merge Queue

Add the label lfg to this PR to add it to the merge queue.

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has enabled the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

This stack of pull requests is managed by Graphite. Learn more about stacking.

@ogp-weeloong ogp-weeloong changed the title feat(test-step): route testRunMetadata to action testRun handlers [SAFE-EMAIL-TEST-REDUX-1] route testRunMetadata to action testRun handlers Jun 4, 2026
@ogp-weeloong ogp-weeloong force-pushed the feat/ses/safe-email-test/test-action-metadata branch from 4d5fe13 to a5d34ab Compare June 4, 2026 07:40
processAction already passes its `metadata` option to both actionCommand.run
and actionCommand.testRun, but test-step.ts never populated that field for
the action path — so testRun handlers always received {}. Forward
options.testRunMetadata as `metadata` so actions can read it.

Safe because metadata's two callers are mutually exclusive: for-each
iteration state is only set during non-test runs (enqueueFirstForEachStep
is gated on !testRun), so the field is free during test runs.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Copy link
Copy Markdown
Contributor Author

currently this doesn't need to be a separate PR but it's cos it had more lines last time before I simplified this

@ogp-weeloong ogp-weeloong force-pushed the feat/ses/safe-email-test/test-action-metadata branch from a5d34ab to 29c6bc9 Compare June 4, 2026 09:02
@ogp-weeloong ogp-weeloong marked this pull request as ready for review June 4, 2026 09:12
@ogp-weeloong ogp-weeloong requested a review from a team as a code owner June 4, 2026 09:12
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.

1 participant