Conversation
| ) { | ||
| // No-op: Notification functionality has been moved to another service | ||
| // TODO: come back to this | ||
| return Promise.resolve() |
There was a problem hiding this comment.
One of these output services could be the one that drops it on SNS?
There was a problem hiding this comment.
Hmm. My thinking is that we could provide an outputService in Runner that we could inject into the plugin and override it which I suppose has a cleaner separation of concerns and Runner still controls its own logic. Happy to discuss the approach if there's a better alternative.
There was a problem hiding this comment.
having in engine would be nice if other teams wanted to use it, great idea to the use the output service though.
| timestamp: now.toISOString(), | ||
| referenceNumber: context.referenceNumber, | ||
| definition: model.def | ||
| formId: model.def.name, |
There was a problem hiding this comment.
we need to leave v1 and v2 and add a new output formatter. I don't know where you can getformId from, but it is the FormMetadata.id: model/src/form/form-metadata/types.ts line 101
There was a problem hiding this comment.
Good point. We're creating a separate output service that will use the machine v2 formatter.
There was a problem hiding this comment.
I think we'll need to leave v2 intact, as our schema will change slightly - removing definition would be a breaking change. The new output service can use the new output formatter
mokhld
force-pushed
the
feat-listener-integration
branch
from
e6bd218 to
993e6ce
Compare
mokhld
force-pushed
the
feat-listener-integration
branch
from
993e6ce to
d7f4b93
Compare
mokhld
force-pushed
the
feat-listener-integration
branch
from
34683f0 to
7289e09
Compare
mokhld
force-pushed
the
feat-listener-integration
branch
from
90368d3 to
cf23a18
Compare
| '2': formatMachineV2 | ||
| }, | ||
| adapter: { | ||
| '1': formatMachineV3 |
There was a problem hiding this comment.
would it be worth changing this name to formatAdapterV1 to avoid confusion?
There was a problem hiding this comment.
Good shout. I've created a new folder under /ouputFormatters/adapter/v1.ts to be more descriptive.
mokhld
force-pushed
the
feat-listener-integration
branch
from
655592e to
383900c
Compare
mokhld
changed the title
There was a problem hiding this comment.
Looks great, thanks for the coverage :). Just question about where to whether to put SNS publishing here or in runner, but will leave this to @alexluckett
| formMetadata?: FormMetadata | ||
| ) { | ||
| if (!templateId) { | ||
| return Promise.resolve() |
There was a problem hiding this comment.
any reason we're resolving the promise here, not throwing?
There was a problem hiding this comment.
The service is optional and should silently fail if it isn't configured.
|
3 participants
Proposed change
Jira ticket: https://eaflood.atlassian.net/browse/DF-388
Type of change
This PR introduces a new adapter V1 formatter and relevant types to be consumed by Runner and the Adapter.
Checklist
README.mdanddocs/*(where appropriate, e.g. new features).npm run test).npm run lint).npm run format).