feat: support workflow version in run requests#150
feat: support workflow version in run requests#150Kaiser-Wu wants to merge 1 commit intocoze-dev:mainfrom
Conversation
|
|
📝 WalkthroughWalkthroughA new Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
api/src/test/java/com/coze/openapi/service/service/workflow/WorkFlowRunServiceTest.java (1)
175-177: Prefer key-presence assertions beforeasText()for clearer failures.At Line 175–Line 177,
json.get(...).asText()can NPE when a field is absent; asserting key existence first makes failures explicit and easier to diagnose.Proposed test assertion refinement
- assertEquals("1.2.3", json.get("workflow_version").asText()); - assertEquals("test_workflow_id", json.get("workflow_id").asText()); - assertEquals("test_app_id", json.get("app_id").asText()); + assertTrue(json.hasNonNull("workflow_version")); + assertTrue(json.hasNonNull("workflow_id")); + assertTrue(json.hasNonNull("app_id")); + assertEquals("1.2.3", json.get("workflow_version").asText()); + assertEquals("test_workflow_id", json.get("workflow_id").asText()); + assertEquals("test_app_id", json.get("app_id").asText());🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@api/src/test/java/com/coze/openapi/service/service/workflow/WorkFlowRunServiceTest.java` around lines 175 - 177, Add explicit key-presence checks on the JSON node before calling asText(): before the assertions that use json.get(...).asText() (the three lines referencing "workflow_version", "workflow_id", and "app_id" in WorkFlowRunServiceTest where the variable json is used), add assertions such as assertTrue(json.has("workflow_version")), assertTrue(json.has("workflow_id")), and assertTrue(json.has("app_id")) so failures indicate a missing key rather than causing an NPE when asText() is called.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@api/src/test/java/com/coze/openapi/service/service/workflow/WorkFlowRunServiceTest.java`:
- Around line 175-177: Add explicit key-presence checks on the JSON node before
calling asText(): before the assertions that use json.get(...).asText() (the
three lines referencing "workflow_version", "workflow_id", and "app_id" in
WorkFlowRunServiceTest where the variable json is used), add assertions such as
assertTrue(json.has("workflow_version")), assertTrue(json.has("workflow_id")),
and assertTrue(json.has("app_id")) so failures indicate a missing key rather
than causing an NPE when asText() is called.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 2b085868-7cdc-4bd6-881f-a5b7ded7e940
📒 Files selected for processing (2)
api/src/main/java/com/coze/openapi/client/workflows/run/RunWorkflowReq.javaapi/src/test/java/com/coze/openapi/service/service/workflow/WorkFlowRunServiceTest.java
|
Superseded by a dedicated workflow fix PR. |
What changed
workflowVersiontoRunWorkflowReqworkflow_versionworkflow_versionWhy
The workflow run API supports
workflow_version, but the Java SDK request model did not expose it, so callers could not specify a workflow version.Validation
env JAVA_HOME=/Library/Java/JavaVirtualMachines/zulu-17.jdk/Contents/Home PATH=/Library/Java/JavaVirtualMachines/zulu-17.jdk/Contents/Home/bin:$PATH mvn -Dmaven.repo.local=/tmp/coze-java-m2 -pl api -Dtest=WorkFlowRunServiceTest test