Switch Mockoon URLs to localhost and add GetInstances test #12
Conversation
Replaced all 'mockoon:3001' URLs with 'localhost:3001' in task and workflow files for local development compatibility. Added new GetInstances task, mapping, and test to support instance filtering and pagination in the task-test workflow. Updated error boundary workflow transitions and initial transition mapping to include the new test.
Reviewer's GuideSwitches all Mockoon-based HTTP URLs to localhost for local development and introduces a new GetInstances test task and mapping to validate instance filtering/pagination, wiring it into the task-test and error-boundary workflows and adjusting GetInstanceData response handling accordingly. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughThe PR updates API endpoint references across task configurations and workflows from a Mockoon service host to localhost, introduces a new GetInstances task with mapping logic for workflow instance retrieval, adjusts error handling transitions, modifies data extraction paths, and expands Postman collection test scenarios with additional workflow steps. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (43)
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 @yilmaztayfun, 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 improves the local development experience by standardizing Mockoon endpoint URLs to Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. 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 - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `core/Workflows/task-test/src/GetInstancesMapping.csx:144-150` </location>
<code_context>
+ if (isSuccess)
+ {
+ // Extract response data
+ var instances = response?.data.items;
+
+ LogInformation("GetInstances Test - Successfully retrieved {0} instance(s)",
+ args: new object?[] { instances?.Count ?? 0 });
+
+ // Build instance summary for verification
+ var instanceSummary = new List<object>();
+ if (instances != null)
+ {
+ foreach (var instance in instances)
+ {
+ instanceSummary.Add(new
+ {
+ testId = instance?.data?.testId
</code_context>
<issue_to_address>
**suggestion (testing):** Expand the instance summary to validate that filters are actually applied (e.g., parentInstanceId, startedBy) rather than only echoing testId.
Right now `instanceSummary` only includes `testId`, so the test can’t verify that the `parentInstanceId` and `startedBy` filters were respected. Please include at least `parentInstanceId`, `testId`, and `startedBy` (and optionally the instance ID) so the `getInstancesResult` output can be checked against all requested filters, not just that instances were returned.
```suggestion
foreach (var instance in instances)
{
instanceSummary.Add(new
{
instanceId = instance?.id,
parentInstanceId = instance?.parentInstanceId,
startedBy = instance?.startedBy,
testId = instance?.data?.testId
});
}
```
</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 effectively standardizes the Mockoon URLs to localhost for better local development consistency and introduces a new GetInstances task to test instance filtering and pagination. The workflow and test updates are well-integrated. I've identified a potential null reference exception in the new C# mapping script that should be addressed. Additionally, I have a couple of suggestions to improve the naming of new requests in the Postman collection for better clarity.
| var parentInstanceId = context.Instance.Id.ToString(); | ||
| var testId = context.Instance.Data?.testId.ToString(); |
There was a problem hiding this comment.
There's a potential NullReferenceException in this block. context.Instance could be null, which would cause context.Instance.Id to throw an exception. Similarly, context.Instance.Data?.testId could evaluate to null, and calling .ToString() on it would also fail. To prevent this, you should use the null-conditional operator ?. for safer property access, which is a common practice in other mapping files in this repository.
var parentInstanceId = context.Instance?.Id.ToString();
var testId = context.Instance?.Data?.testId?.ToString();
| "response": [] | ||
| }, | ||
| { | ||
| "name": "Get Instances Copy", |
There was a problem hiding this comment.
The name of this new Postman request, "Get Instances Copy", is misleading because the request targets the .../functions/schema endpoint, which retrieves a workflow schema, not instances. For better clarity and maintainability of the collection, I recommend renaming it to something more descriptive, such as "Get Workflow Schema".
| "response": [] | ||
| }, | ||
| { | ||
| "name": "Get Instances", |
There was a problem hiding this comment.
This request is named "Get Instances", but the URL .../functions/data appears to fetch data for a single instance rather than a list of instances. This could be confusing for others using the collection. Renaming it to "Get Instance Data" would more accurately describe its purpose.
Replaced all 'mockoon:3001' URLs with 'localhost:3001' in task and workflow files for local development compatibility. Added new GetInstances task, mapping, and test to support instance filtering and pagination in the task-test workflow. Updated error boundary workflow transitions and initial transition mapping to include the new test.
Summary by Sourcery
Add a GetInstances test flow and align instance data mappings while updating local development endpoints.
New Features:
Enhancements:
Build:
Tests:
Summary by CodeRabbit
Release Notes
Chores
New Features
Tests
✏️ Tip: You can customize this high-level summary in your review settings.