Example Trigger - #1
Conversation
Reviewer's GuideAdds new account-opening workflow tasks and mappings to trigger a scheduled-payments subprocess and to fetch data from an existing workflow instance, wiring them into the account-opening workflow and Postman collection. Sequence diagram for triggering scheduled-payments subprocess from account-opening workflowsequenceDiagram
actor User
participant AccountOpeningWorkflowEngine
participant TriggerScheduledPaymentsMapping
participant SubProcessTask
participant ScheduledPaymentsWorkflowEngine
User->>AccountOpeningWorkflowEngine: Start account-opening workflow
AccountOpeningWorkflowEngine->>TriggerScheduledPaymentsMapping: Invoke InputHandler(task, context)
TriggerScheduledPaymentsMapping->>SubProcessTask: SetDomain(core)
TriggerScheduledPaymentsMapping->>SubProcessTask: SetKey(scheduled-payments)
TriggerScheduledPaymentsMapping->>SubProcessTask: SetVersion(1.0.0)
TriggerScheduledPaymentsMapping->>SubProcessTask: SetBody(userId, amount, currency, frequency, dates, paymentMethodId, description, recipientId, isAutoRetry, maxRetries)
TriggerScheduledPaymentsMapping-->>AccountOpeningWorkflowEngine: ScriptResponse(Data = context.Instance.Data)
AccountOpeningWorkflowEngine->>ScheduledPaymentsWorkflowEngine: Start subprocess(core, scheduled-payments, 1.0.0, body)
ScheduledPaymentsWorkflowEngine-->>AccountOpeningWorkflowEngine: Acknowledge started
AccountOpeningWorkflowEngine->>TriggerScheduledPaymentsMapping: Invoke OutputHandler(context)
TriggerScheduledPaymentsMapping-->>AccountOpeningWorkflowEngine: ScriptResponse(Data.scheduledPaymentsInitiated = true, Data.status = SCHEDULED_PAYMENTS_SUBPROCESS_LAUNCHED)
Sequence diagram for getting data from an existing workflow instancesequenceDiagram
participant AccountOpeningWorkflowEngine
participant TriggerGetInstanceTaskMapping
participant GetInstanceDataTask
participant WorkflowRuntime
participant OldWorkflowInstance
AccountOpeningWorkflowEngine->>TriggerGetInstanceTaskMapping: Invoke InputHandler(task, context)
TriggerGetInstanceTaskMapping->>GetInstanceDataTask: SetInstance(context.Instance.Data.oldInstanceId)
TriggerGetInstanceTaskMapping-->>AccountOpeningWorkflowEngine: ScriptResponse()
AccountOpeningWorkflowEngine->>WorkflowRuntime: Execute GetInstanceDataTask
WorkflowRuntime->>OldWorkflowInstance: Fetch instance data(oldInstanceId)
OldWorkflowInstance-->>WorkflowRuntime: Instance data
WorkflowRuntime-->>AccountOpeningWorkflowEngine: Task result(body = old instance data)
AccountOpeningWorkflowEngine->>TriggerGetInstanceTaskMapping: Invoke OutputHandler(context)
TriggerGetInstanceTaskMapping-->>AccountOpeningWorkflowEngine: ScriptResponse(Data.oldInstance = context.Body, Data.success = true)
Class diagram for new account-opening mapping scriptsclassDiagram
class IMapping {
<<interface>>
+InputHandler(task: WorkflowTask, context: ScriptContext) Task~ScriptResponse~
+OutputHandler(context: ScriptContext) Task~ScriptResponse~
}
class ScriptBase {
}
class WorkflowTask {
}
class SubProcessTask {
+SetDomain(domain: string) void
+SetKey(key: string) void
+SetVersion(version: string) void
+SetBody(body: object) void
}
class GetInstanceDataTask {
+SetInstance(instanceId: string) void
}
class ScriptContext {
+Instance InstanceReference
+Body object
}
class InstanceReference {
+Data dynamic
}
class ScriptResponse {
+Data object
}
class TriggerScheduledPaymentsMapping {
+InputHandler(task: WorkflowTask, context: ScriptContext) Task~ScriptResponse~
+OutputHandler(context: ScriptContext) Task~ScriptResponse~
}
class TriggerGetInstanceTaskMapping {
+InputHandler(task: WorkflowTask, context: ScriptContext) Task~ScriptResponse~
+OutputHandler(context: ScriptContext) Task~ScriptResponse~
}
class RequestModel {
+key string
+attributes dynamic
}
IMapping <|.. TriggerScheduledPaymentsMapping
IMapping <|.. TriggerGetInstanceTaskMapping
ScriptBase <|-- TriggerScheduledPaymentsMapping
WorkflowTask <|-- SubProcessTask
WorkflowTask <|-- GetInstanceDataTask
ScriptContext o-- InstanceReference
ScriptResponse o-- TriggerScheduledPaymentsMapping
ScriptResponse o-- TriggerGetInstanceTaskMapping
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
WalkthroughA new account-opening workflow enhancement is implemented with task configurations for data retrieval and scheduled payments handling. Two C# mapping scripts orchestrate data flow between workflow tasks and subprocesses. Workflow transitions now include execution tasks that invoke these mappings. A Postman collection is updated with corresponding test requests. Changes
Sequence DiagramsequenceDiagram
participant WF as Account-Opening Workflow
participant GetData as get-data-from-workflow Task
participant SubProc as execute-sub Subprocess
participant SchedPay as trigger-scheduled-payments Task
participant Context as Workflow Context
WF->>GetData: onExecutionTasks (startTransition)
GetData->>Context: Retrieve oldInstanceId
Context-->>GetData: Instance Data
GetData->>WF: Return oldInstance + success
WF->>WF: account-type-selection transition
WF->>GetData: onExecutionTasks (re-fetch)
GetData-->>WF: Workflow data
WF->>SubProc: execute-sub state
SubProc->>SchedPay: onExecutionTasks trigger
SchedPay->>SchedPay: Configure subprocess<br/>(domain, key, version)
SchedPay->>SchedPay: Initialize body fields<br/>(userId, amount, currency, etc.)
SchedPay-->>SubProc: Fire-and-forget ack<br/>(SCHEDULED_PAYMENTS_SUBPROCESS_LAUNCHED)
SubProc-->>WF: Subprocess initiated
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary of ChangesHello @tsimsekburgan, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly extends the functionality of the 'account-opening' workflow by introducing new system tasks for data handling and subprocess orchestration. It allows the workflow to retrieve specific instance data and to initiate a 'scheduled-payments' subprocess with pre-defined parameters, streamlining complex financial operations. The changes are supported by new C# mapping scripts that ensure proper data flow and task configuration, and the updated Postman collection facilitates thorough testing of these new features. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- In
TriggerScheduledPaymentsMapping.InputHandler, all subprocess body fields are hard-coded (including IDs, amount, and dates); consider sourcing these fromtask/contextor configuration so the mapping is reusable and avoids baking in environment-specific values, and alignendDatetype withstartDate(both asDateTimeor both as strings). TriggerScheduledPaymentsMapping.OutputHandleris declaredasyncbut never awaits anything and just returns a prebuilt response; you can simplify this by removingasync, returningTask.FromResult, and cleaning up the extra indentation aroundresponse.Dataassignment.- In
TriggerGetInstanceTaskMapping, the null-forgiving cast(task as GetInstanceDataTask)!and direct use ofcontext.Instance.Data.oldInstanceIdassume specific types and presence; add explicit type/shape validation (or a clear exception) and remove unused items likeSystem.Text.Jsonand theRequestModelclass to keep the mapping safe and focused.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `TriggerScheduledPaymentsMapping.InputHandler`, all subprocess body fields are hard-coded (including IDs, amount, and dates); consider sourcing these from `task`/`context` or configuration so the mapping is reusable and avoids baking in environment-specific values, and align `endDate` type with `startDate` (both as `DateTime` or both as strings).
- `TriggerScheduledPaymentsMapping.OutputHandler` is declared `async` but never awaits anything and just returns a prebuilt response; you can simplify this by removing `async`, returning `Task.FromResult`, and cleaning up the extra indentation around `response.Data` assignment.
- In `TriggerGetInstanceTaskMapping`, the null-forgiving cast `(task as GetInstanceDataTask)!` and direct use of `context.Instance.Data.oldInstanceId` assume specific types and presence; add explicit type/shape validation (or a clear exception) and remove unused items like `System.Text.Json` and the `RequestModel` class to keep the mapping safe and focused.
## Individual Comments
### Comment 1
<location> `core/Workflows/account-opening/src/TriggerScheduledPaymentsMapping.csx:45-60` </location>
<code_context>
+ });
+ }
+
+ public async Task<ScriptResponse> OutputHandler(ScriptContext context)
+ {
+ var response = new ScriptResponse();
+
+ // SubProcess is fire-and-forget
+ // Just track that it was initiated
+
+ response.Data = new
+ {
+ scheduledPaymentsInitiated = true,
+ initiatedAt = DateTime.UtcNow,
+ status = "SCHEDULED_PAYMENTS_SUBPROCESS_LAUNCHED"
+ };
+
+ return response;
+ }
+}
</code_context>
<issue_to_address>
**suggestion:** Remove `async` or introduce `await` to avoid an unnecessary async state machine.
Since this method only returns a precomputed `ScriptResponse` and performs no asynchronous work, the `async` keyword just adds an unnecessary state machine and may trigger analyzer warnings. Either remove `async` and return `Task.FromResult(response)`, or make the body truly asynchronous if you expect async operations here later.
```suggestion
public Task<ScriptResponse> OutputHandler(ScriptContext context)
{
var response = new ScriptResponse();
// SubProcess is fire-and-forget
// Just track that it was initiated
response.Data = new
{
scheduledPaymentsInitiated = true,
initiatedAt = DateTime.UtcNow,
status = "SCHEDULED_PAYMENTS_SUBPROCESS_LAUNCHED"
};
return Task.FromResult(response);
}
```
</issue_to_address>
### Comment 2
<location> `core/Workflows/account-opening/src/TriggerGetInstanceTaskMapping.csx:9` </location>
<code_context>
+{
+ public Task<ScriptResponse> InputHandler(WorkflowTask task, ScriptContext context)
+ {
+ var triggerTask = (task as GetInstanceDataTask)!;
+
+ triggerTask.SetInstance(context.Instance.Data.oldInstanceId);
</code_context>
<issue_to_address>
**issue (bug_risk):** Avoid combining `as` with null-forgiving when casting `WorkflowTask`.
`(task as GetInstanceDataTask)!` will throw a `NullReferenceException` later if `task` is not that type, and the null-forgiving operator hides this from the compiler. If a mismatch should be a hard error, prefer `(GetInstanceDataTask)task` so it fails immediately. If it might legitimately be another type, keep `as` but handle the possible null explicitly.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces new workflow integrations for triggering scheduled payments and retrieving data from other workflow instances. The implementation is mostly sound, but I've identified several areas for improvement. Key issues include redundant inline code in workflow definitions which impacts maintainability, a potential null reference exception in a C# mapping script, and significant use of hardcoded values in another script which should be dynamic. I've also noted a couple of minor typos in the Postman collection. Addressing these points will improve the code's robustness, maintainability, and correctness.
| { | ||
| public Task<ScriptResponse> InputHandler(WorkflowTask task, ScriptContext context) | ||
| { | ||
| var triggerTask = (task as GetInstanceDataTask)!; |
There was a problem hiding this comment.
Using the null-forgiving operator ! can hide potential issues. If the task object cannot be cast to GetInstanceDataTask, triggerTask will be null, and the next line will throw a NullReferenceException. It's safer to perform an explicit null check and throw a more descriptive exception.
var triggerTask = task as GetInstanceDataTask ?? throw new InvalidOperationException("Task must be a GetInstanceDataTask.");
| subProcessTask.SetBody(new | ||
| { | ||
| userId = 1, | ||
| amount = 12000, | ||
| currency = "TL", | ||
| frequency = "monthly", | ||
| startDate = DateTime.UtcNow, | ||
| endDate = "2026-10-01T09:02:38.201Z", | ||
| paymentMethodId = "1", | ||
| description = "X ödeme", | ||
| recipientId = "324324", | ||
| isAutoRetry = false, | ||
| maxRetries = 3 | ||
| }); |
There was a problem hiding this comment.
The body for the subprocess is populated with hardcoded example data. The comment on line 23 indicates that this data should be passed from the parent workflow. Please replace these hardcoded values with dynamic data from the context object to make this script functional in a real-world scenario. For example, userId could potentially be retrieved from context.Instance.Data.userSession.userId or a similar property.
| }, | ||
| "mapping": { | ||
| "location": "./src/TriggerGetInstanceTaskMapping.csx", | ||
| "code": "dXNpbmcgU3lzdGVtLlRocmVhZGluZy5UYXNrczsKdXNpbmcgQkJULldvcmtmbG93LlNjcmlwdGluZzsKdXNpbmcgU3lzdGVtLlRleHQuSnNvbjsKCnB1YmxpYyBjbGFzcyBUcmlnZ2VyR2V0SW5zdGFuY2VUYXNrTWFwcGluZyA6IElNYXBwaW5nCnsKICAgIHB1YmxpYyBUYXNrPFNjcmlwdFJlc3BvbnNlPiBJbnB1dEhhbmRsZXIoV29ya2Zsb3dUYXNrIHRhc2ssIFNjcmlwdENvbnRleHQgY29udGV4dCkKICAgIHsKICAgICAgICB2YXIgdHJpZ2dlclRhc2sgPSAodGFzayBhcyBHZXRJbnN0YW5jZURhdGFUYXNrKSE7CiAgICAgICAKICAgICAgICB0cmlnZ2VyVGFzay5TZXRJbnN0YW5jZShjb250ZXh0Lkluc3RhbmNlLkRhdGEub2xkSW5zdGFuY2VJZCk7CiAgICAgICAgcmV0dXJuIFRhc2suRnJvbVJlc3VsdChuZXcgU2NyaXB0UmVzcG9uc2UoKSk7CiAgICB9CgogICAgcHVibGljIFRhc2s8U2NyaXB0UmVzcG9uc2U+IE91dHB1dEhhbmRsZXIoU2NyaXB0Q29udGV4dCBjb250ZXh0KQogICAgewoKICAgICAgICByZXR1cm4gVGFzay5Gcm9tUmVzdWx0KG5ldyBTY3JpcHRSZXNwb25zZSgpCiAgICAgICAgewogICAgICAgICAgICBEYXRhID0gbmV3CiAgICAgICAgICAgIHsKICAgICAgICAgICAgICAgIG9sZEluc3RhbmNlPWNvbnRleHQuQm9keSwKICAgICAgICAgICAgICAgIHN1Y2Nlc3MgPSB0cnVlCiAgICAgICAgICAgIH0KICAgICAgICB9KTsKICAgIH0KfQoKcHVibGljIGNsYXNzIFJlcXVlc3RNb2RlbAp7CiAgICBwdWJsaWMgc3RyaW5nIGtleSB7IGdldDsgc2V0OyB9CiAgICBwdWJsaWMgZHluYW1pYyAgYXR0cmlidXRlcyB7IGdldDsgc2V0OyB9Cn0=" |
There was a problem hiding this comment.
The code property is redundant because the script is already referenced by the location property on the previous line. Including the encoded script here can lead to maintenance issues if the .csx file is updated but this code field is not, causing the old script to be executed. Please remove the code property to ensure the script from the file is always used.
| }, | ||
| "mapping": { | ||
| "location": "./src/TriggerScheduledPaymentsMapping.csx", | ||
| "code": "dXNpbmcgU3lzdGVtOwp1c2luZyBTeXN0ZW0uVGhyZWFkaW5nLlRhc2tzOwp1c2luZyBCQlQuV29ya2Zsb3cuU2NyaXB0aW5nOwp1c2luZyBCQlQuV29ya2Zsb3cuRGVmaW5pdGlvbnM7CgpwdWJsaWMgY2xhc3MgVHJpZ2dlclNjaGVkdWxlZFBheW1lbnRzTWFwcGluZyA6U2NyaXB0QmFzZSwgSU1hcHBpbmcKewogICAgcHVibGljIFRhc2s8U2NyaXB0UmVzcG9uc2U+IElucHV0SGFuZGxlcihXb3JrZmxvd1Rhc2sgdGFzaywgU2NyaXB0Q29udGV4dCBjb250ZXh0KQogICAgewogICAgICAgIHZhciBzdWJQcm9jZXNzVGFzayA9IHRhc2sgYXMgU3ViUHJvY2Vzc1Rhc2s7CgogICAgICAgIGlmIChzdWJQcm9jZXNzVGFzayA9PSBudWxsKQogICAgICAgIHsKICAgICAgICAgICAgdGhyb3cgbmV3IEludmFsaWRPcGVyYXRpb25FeGNlcHRpb24oIlRhc2sgbXVzdCBiZSBhIFN1YlByb2Nlc3NUYXNrIik7CiAgICAgICAgfQoKICAgICAgICAvLyBDb25maWd1cmUgc3VicHJvY2VzcwogICAgICAgIHN1YlByb2Nlc3NUYXNrLlNldERvbWFpbigiY29yZSIpOwogICAgICAgIHN1YlByb2Nlc3NUYXNrLlNldEtleSgic2NoZWR1bGVkLXBheW1lbnRzIik7CiAgICAgICAgc3ViUHJvY2Vzc1Rhc2suU2V0VmVyc2lvbigiMS4wLjAiKTsKCiAgICAgICAgLy8gUHJlcGFyZSBzdWJwcm9jZXNzIGluaXRpYWxpemF0aW9uIGRhdGEKICAgICAgICAvLyBQYXNzIHJlbGV2YW50IGRhdGEgZnJvbSBhY2NvdW50LW9wZW5pbmcgd29ya2Zsb3cgdG8gc2NoZWR1bGVkLXBheW1lbnRzCiAgICAgICAgc3ViUHJvY2Vzc1Rhc2suU2V0Qm9keShuZXcKICAgICAgICB7CiAgICAgICAgICAgIHVzZXJJZCA9IDEsCiAgICAgICAgICAgIGFtb3VudCA9IDEyMDAwLAogICAgICAgICAgICBjdXJyZW5jeSA9ICJUTCIsCiAgICAgICAgICAgIGZyZXF1ZW5jeSA9ICJtb250aGx5IiwKICAgICAgICAgICAgc3RhcnREYXRlID0gRGF0ZVRpbWUuVXRjTm93LAogICAgICAgICAgICBlbmREYXRlID0gIjIwMjYtMTAtMDFUMDk6MDI6MzguMjAxWiIsCiAgICAgICAgICAgIHBheW1lbnRNZXRob2RJZCA9ICIxIiwKICAgICAgICAgICAgZGVzY3JpcHRpb24gPSAiWCDDtmRlbWUiLAogICAgICAgICAgICByZWNpcGllbnRJZCA9ICIzMjQzMjQiLAogICAgICAgICAgICBpc0F1dG9SZXRyeSA9IGZhbHNlLAogICAgICAgICAgICBtYXhSZXRyaWVzID0gMwogICAgICAgIH0pOwoKICAgICAgICByZXR1cm4gVGFzay5Gcm9tUmVzdWx0KG5ldyBTY3JpcHRSZXNwb25zZQogICAgICAgIHsKICAgICAgICAgICAgRGF0YSA9IGNvbnRleHQuSW5zdGFuY2U/LkRhdGEKICAgICAgICB9KTsKICAgIH0KCiAgICBwdWJsaWMgYXN5bmMgVGFzazxTY3JpcHRSZXNwb25zZT4gT3V0cHV0SGFuZGxlcihTY3JpcHRDb250ZXh0IGNvbnRleHQpCiAgICB7CiAgICAgICAgdmFyIHJlc3BvbnNlID0gbmV3IFNjcmlwdFJlc3BvbnNlKCk7CgogICAgICAgIC8vIFN1YlByb2Nlc3MgaXMgZmlyZS1hbmQtZm9yZ2V0CiAgICAgICAgLy8gSnVzdCB0cmFjayB0aGF0IGl0IHdhcyBpbml0aWF0ZWQKICAgICAgCiAgICAgICAgICAgIHJlc3BvbnNlLkRhdGEgPSBuZXcKICAgICAgICAgICAgewogICAgICAgICAgICAgICAgc2NoZWR1bGVkUGF5bWVudHNJbml0aWF0ZWQgPSB0cnVlLAogICAgICAgICAgICAgICAgaW5pdGlhdGVkQXQgPSBEYXRlVGltZS5VdGNOb3csCiAgICAgICAgICAgICAgICBzdGF0dXMgPSAiU0NIRURVTEVEX1BBWU1FTlRTX1NVQlBST0NFU1NfTEFVTkNIRUQiCiAgICAgICAgICAgIH07CgogICAgICAgIHJldHVybiByZXNwb25zZTsKICAgIH0KfQoK" |
There was a problem hiding this comment.
The code property is redundant because the script is already referenced by the location property on the previous line. Including the encoded script here can lead to maintenance issues if the .csx file is updated but this code field is not, causing the old script to be executed. Please remove the code property to ensure the script from the file is always used.
| public string key { get; set; } | ||
| public dynamic attributes { get; set; } |
There was a problem hiding this comment.
According to C# naming conventions, public properties should use PascalCase. Please rename key to Key and attributes to Attributes for consistency and readability.
public string Key { get; set; }
public dynamic Attributes { get; set; }
| subProcessTask.SetDomain("core"); | ||
| subProcessTask.SetKey("scheduled-payments"); | ||
| subProcessTask.SetVersion("1.0.0"); |
There was a problem hiding this comment.
The domain, key, and version for the subprocess are hardcoded. These values are already defined in the trigger-scheduled-payments.json task definition. To improve maintainability and reusability, you should retrieve these values from the task's configuration instead of hardcoding them.
For example:
// Assuming the config is available on the task object
subProcessTask.SetDomain(subProcessTask.Configuration.domain);
subProcessTask.SetKey(subProcessTask.Configuration.key);
subProcessTask.SetVersion(subProcessTask.Configuration.version);(Note: The exact API to get the configuration might differ, but the principle is to avoid hardcoding.)
| "body": { | ||
| "mode": "raw", | ||
| "raw": "{\n \"key\": \"3498749106090045454232333\",\n \"tags\": [\n \"test\",\n \"banking\",\n \"account-openning\"\n ],\n \"attributes\": {\n \"session\": \"16\"\n }\n}", | ||
| "raw": "{\n \"key\": \"3498749106090045454232333\",\n \"tags\": [\n \"test\",\n \"banking\",\n \"account-openning\"\n ],\n \"attributes\": {\n \"session\": \"16\",\n \"oldInstanceId\": \"3498749106090045454232333\"\n }\n}", |
There was a problem hiding this comment.
There is a typo in the tag "account-openning". It should be "account-opening". Correcting this will ensure consistency and prevent potential issues with tag-based queries.
| "raw": "{\n \"key\": \"3498749106090045454232333\",\n \"tags\": [\n \"test\",\n \"banking\",\n \"account-openning\"\n ],\n \"attributes\": {\n \"session\": \"16\",\n \"oldInstanceId\": \"3498749106090045454232333\"\n }\n}", | |
| "raw": "{\n \"key\": \"3498749106090045454232333\",\n \"tags\": [\n \"test\",\n \"banking\",\n \"account-opening\"\n ],\n \"attributes\": {\n \"session\": \"16\",\n \"oldInstanceId\": \"3498749106090045454232333\"\n }\n}", |
| "response": [] | ||
| }, | ||
| { | ||
| "name": "Step 1.5(optinal): Execute Sub Copy", |
There was a problem hiding this comment.
There is a typo in the request name: "optinal" should be "optional".
| "name": "Step 1.5(optinal): Execute Sub Copy", | |
| "name": "Step 1.5 (optional): Execute Sub Copy", |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
core/Workflows/account-opening/src/TriggerGetInstanceTaskMapping.csx (1)
29-33: Remove unused RequestModel class.The
RequestModelclass is defined but never used in this file. Consider removing it to reduce code clutter, or clarify its intended purpose with a comment if it's meant for future use.Apply this diff to remove the unused class:
}); } } - -public class RequestModel -{ - public string key { get; set; } - public dynamic attributes { get; set; } -}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
core/Tasks/account-opening/get-data-from-workflow.json(1 hunks)core/Tasks/account-opening/trigger-scheduled-payments.json(1 hunks)core/Workflows/account-opening/account-opening-workflow.json(1 hunks)core/Workflows/account-opening/src/TriggerGetInstanceTaskMapping.csx(1 hunks)core/Workflows/account-opening/src/TriggerScheduledPaymentsMapping.csx(1 hunks)postman/vNext Example Runtime.postman_collection.json(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Sourcery review
🔇 Additional comments (6)
postman/vNext Example Runtime.postman_collection.json (1)
730-730: LGTM! Test data addition aligns with workflow mapping.The
oldInstanceIdattribute in the test data properly supports the new workflow data retrieval feature implemented inTriggerGetInstanceTaskMapping.csx.core/Tasks/account-opening/get-data-from-workflow.json (1)
1-15: LGTM! Task configuration is correct.The task configuration properly defines a workflow data retrieval task. The empty
instanceIdat line 12 is intentional as it will be populated at runtime by theTriggerGetInstanceTaskMapping.csxmapping via theSetInstance()method usingcontext.Instance.Data.oldInstanceId.core/Tasks/account-opening/trigger-scheduled-payments.json (1)
1-22: LGTM! Subprocess trigger configuration is correct.The configuration properly defines a subprocess trigger task with appropriate tags and references to the
scheduled-paymentssubprocess. The structure aligns well with theTriggerScheduledPaymentsMapping.csximplementation.core/Workflows/account-opening/account-opening-workflow.json (2)
125-139: LGTM! Execution task integration is correct.The
onExecutionTasksproperly integrates the workflow data retrieval task using theTriggerGetInstanceTaskMapping.csxmapping. This enables fetching instance data during the account type selection transition.
141-184: The execute-sub self-loop transition is intentional and safe—no action required.The
execute-subtransition targetingaccount-type-selectionis a deliberate pattern, not a problematic loop. Analysis shows:
- Fire-and-forget design: The subprocess executes asynchronously (
onExecutionTasks), and the OutputHandler returns immediately without blocking- User control: From
account-type-selection, users can either triggerexecute-sub(which keeps them on the same state after launching the subprocess) or proceed viaselect-demand-deposittoaccount-details-input- Safe state machine: Self-loop transitions are a valid workflow pattern for triggering side-effect actions while remaining on the current screen, allowing users to proceed when ready
No issues found. The workflow design is sound.
core/Workflows/account-opening/src/TriggerScheduledPaymentsMapping.csx (1)
45-60: LGTM! Fire-and-forget acknowledgement pattern is appropriate.The
OutputHandlercorrectly implements a fire-and-forget pattern for subprocess initiation, returning an acknowledgement that tracks the initiation timestamp and status. This is appropriate for asynchronous subprocess execution.
| public Task<ScriptResponse> InputHandler(WorkflowTask task, ScriptContext context) | ||
| { | ||
| var triggerTask = (task as GetInstanceDataTask)!; | ||
|
|
||
| triggerTask.SetInstance(context.Instance.Data.oldInstanceId); | ||
| return Task.FromResult(new ScriptResponse()); | ||
| } |
There was a problem hiding this comment.
Add null safety checks to prevent runtime exceptions.
The code has multiple null safety issues:
- Line 9: The null-forgiving operator
!suppresses compiler warnings but doesn't preventNullReferenceExceptionif the cast fails. - Line 11: Chained property access
context.Instance.Data.oldInstanceIdlacks null checks at each level.
These issues could cause runtime exceptions if:
- The task is not a
GetInstanceDataTask - The context, Instance, Data, or oldInstanceId properties are null
Apply this diff to add proper null safety:
public Task<ScriptResponse> InputHandler(WorkflowTask task, ScriptContext context)
{
- var triggerTask = (task as GetInstanceDataTask)!;
+ var triggerTask = task as GetInstanceDataTask;
+
+ if (triggerTask == null)
+ {
+ throw new InvalidOperationException("Task must be a GetInstanceDataTask");
+ }
+
+ var oldInstanceId = context?.Instance?.Data?.oldInstanceId;
+ if (string.IsNullOrEmpty(oldInstanceId?.ToString()))
+ {
+ throw new InvalidOperationException("oldInstanceId is required in context.Instance.Data");
+ }
- triggerTask.SetInstance(context.Instance.Data.oldInstanceId);
+ triggerTask.SetInstance(oldInstanceId);
return Task.FromResult(new ScriptResponse());
}🤖 Prompt for AI Agents
In core/Workflows/account-opening/src/TriggerGetInstanceTaskMapping.csx around
lines 7-13, the code unsafely uses a forced cast and chained property access
which can throw NullReferenceException; update the method to first verify the
incoming task is a GetInstanceDataTask (use an "is" pattern or safe cast and
check for null) and validate context, context.Instance, context.Instance.Data,
and the oldInstanceId before calling SetInstance; if any check fails,
short-circuit and return a failed or no-op ScriptResponse (or log/handle the
error) instead of dereferencing nulls.
| public Task<ScriptResponse> InputHandler(WorkflowTask task, ScriptContext context) | ||
| { | ||
| var subProcessTask = task as SubProcessTask; | ||
|
|
||
| if (subProcessTask == null) | ||
| { | ||
| throw new InvalidOperationException("Task must be a SubProcessTask"); | ||
| } | ||
|
|
||
| // Configure subprocess | ||
| subProcessTask.SetDomain("core"); | ||
| subProcessTask.SetKey("scheduled-payments"); | ||
| subProcessTask.SetVersion("1.0.0"); | ||
|
|
||
| // Prepare subprocess initialization data | ||
| // Pass relevant data from account-opening workflow to scheduled-payments | ||
| subProcessTask.SetBody(new | ||
| { | ||
| userId = 1, | ||
| amount = 12000, | ||
| currency = "TL", | ||
| frequency = "monthly", | ||
| startDate = DateTime.UtcNow, | ||
| endDate = "2026-10-01T09:02:38.201Z", | ||
| paymentMethodId = "1", | ||
| description = "X ödeme", | ||
| recipientId = "324324", | ||
| isAutoRetry = false, | ||
| maxRetries = 3 | ||
| }); | ||
|
|
||
| return Task.FromResult(new ScriptResponse | ||
| { | ||
| Data = context.Instance?.Data | ||
| }); | ||
| } |
There was a problem hiding this comment.
Replace hardcoded subprocess data with context values.
The InputHandler properly validates the task type but then initializes the subprocess with entirely hardcoded values (lines 24-37). This means every subprocess execution will use the same data regardless of the workflow context, making this non-functional for real use cases.
Issues:
- Line 26:
userId = 1is hardcoded - Line 27:
amount = 12000is hardcoded - Line 33:
description = "X ödeme"appears to be a test placeholder - All other fields are hardcoded as well
The subprocess should use data from context.Instance.Data to pass relevant information from the account-opening workflow.
Apply this diff to use dynamic data from context:
// Prepare subprocess initialization data
// Pass relevant data from account-opening workflow to scheduled-payments
+ var workflowData = context?.Instance?.Data;
+
subProcessTask.SetBody(new
{
- userId = 1,
- amount = 12000,
- currency = "TL",
- frequency = "monthly",
- startDate = DateTime.UtcNow,
- endDate = "2026-10-01T09:02:38.201Z",
- paymentMethodId = "1",
- description = "X ödeme",
- recipientId = "324324",
- isAutoRetry = false,
- maxRetries = 3
+ userId = workflowData?.userSession?.userId ?? throw new InvalidOperationException("userId is required"),
+ amount = workflowData?.paymentAmount ?? throw new InvalidOperationException("paymentAmount is required"),
+ currency = workflowData?.currency ?? "TL",
+ frequency = workflowData?.paymentFrequency ?? "monthly",
+ startDate = workflowData?.paymentStartDate ?? DateTime.UtcNow,
+ endDate = workflowData?.paymentEndDate,
+ paymentMethodId = workflowData?.paymentMethodId,
+ description = workflowData?.paymentDescription ?? "Scheduled payment",
+ recipientId = workflowData?.recipientId,
+ isAutoRetry = workflowData?.isAutoRetry ?? false,
+ maxRetries = workflowData?.maxRetries ?? 3
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public Task<ScriptResponse> InputHandler(WorkflowTask task, ScriptContext context) | |
| { | |
| var subProcessTask = task as SubProcessTask; | |
| if (subProcessTask == null) | |
| { | |
| throw new InvalidOperationException("Task must be a SubProcessTask"); | |
| } | |
| // Configure subprocess | |
| subProcessTask.SetDomain("core"); | |
| subProcessTask.SetKey("scheduled-payments"); | |
| subProcessTask.SetVersion("1.0.0"); | |
| // Prepare subprocess initialization data | |
| // Pass relevant data from account-opening workflow to scheduled-payments | |
| subProcessTask.SetBody(new | |
| { | |
| userId = 1, | |
| amount = 12000, | |
| currency = "TL", | |
| frequency = "monthly", | |
| startDate = DateTime.UtcNow, | |
| endDate = "2026-10-01T09:02:38.201Z", | |
| paymentMethodId = "1", | |
| description = "X ödeme", | |
| recipientId = "324324", | |
| isAutoRetry = false, | |
| maxRetries = 3 | |
| }); | |
| return Task.FromResult(new ScriptResponse | |
| { | |
| Data = context.Instance?.Data | |
| }); | |
| } | |
| public Task<ScriptResponse> InputHandler(WorkflowTask task, ScriptContext context) | |
| { | |
| var subProcessTask = task as SubProcessTask; | |
| if (subProcessTask == null) | |
| { | |
| throw new InvalidOperationException("Task must be a SubProcessTask"); | |
| } | |
| // Configure subprocess | |
| subProcessTask.SetDomain("core"); | |
| subProcessTask.SetKey("scheduled-payments"); | |
| subProcessTask.SetVersion("1.0.0"); | |
| // Prepare subprocess initialization data | |
| // Pass relevant data from account-opening workflow to scheduled-payments | |
| var workflowData = context?.Instance?.Data; | |
| subProcessTask.SetBody(new | |
| { | |
| userId = workflowData?.userSession?.userId ?? throw new InvalidOperationException("userId is required"), | |
| amount = workflowData?.paymentAmount ?? throw new InvalidOperationException("paymentAmount is required"), | |
| currency = workflowData?.currency ?? "TL", | |
| frequency = workflowData?.paymentFrequency ?? "monthly", | |
| startDate = workflowData?.paymentStartDate ?? DateTime.UtcNow, | |
| endDate = workflowData?.paymentEndDate, | |
| paymentMethodId = workflowData?.paymentMethodId, | |
| description = workflowData?.paymentDescription ?? "Scheduled payment", | |
| recipientId = workflowData?.recipientId, | |
| isAutoRetry = workflowData?.isAutoRetry ?? false, | |
| maxRetries = workflowData?.maxRetries ?? 3 | |
| }); | |
| return Task.FromResult(new ScriptResponse | |
| { | |
| Data = context.Instance?.Data | |
| }); | |
| } |
🤖 Prompt for AI Agents
In core/Workflows/account-opening/src/TriggerScheduledPaymentsMapping.csx around
lines 8 to 43, the subprocess body is populated with hardcoded test values;
replace those with values taken from context.Instance.Data (with null
checks/defaults) so the subprocess receives real workflow data: read
context.Instance?.Data (or cast to dynamic/dictionary) and map fields userId,
amount, currency, frequency, startDate, endDate, paymentMethodId, description,
recipientId, isAutoRetry, maxRetries into the SetBody call; validate or parse
dates (DateTime/ISO string) and numeric types as needed, and throw or log a
clear error if required fields are missing.
| { | ||
| "name": "Step 1.5(optinal): Execute Sub Copy", |
There was a problem hiding this comment.
Fix typo in request name.
The request name contains a typo: "optinal" should be "optional".
Apply this diff to fix the typo:
{
- "name": "Step 1.5(optinal): Execute Sub Copy",
+ "name": "Step 1.5(optional): Execute Sub Copy",
"request": {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| { | |
| "name": "Step 1.5(optinal): Execute Sub Copy", | |
| { | |
| "name": "Step 1.5(optional): Execute Sub Copy", |
🤖 Prompt for AI Agents
In postman/vNext Example Runtime.postman_collection.json around lines 769 to
770, the request name contains a typo ("optinal"); update the "name" value from
"Step 1.5(optinal): Execute Sub Copy" to "Step 1.5 (optional): Execute Sub Copy"
(ensure spacing and spelling are corrected) so the collection uses the correct
word "optional".
2 participants
Summary by Sourcery
Add account-opening workflow integrations to trigger scheduled payments and retrieve data from an existing workflow instance.
New Features:
Summary by CodeRabbit
New Features
Tests
✏️ Tip: You can customize this high-level summary in your review settings.