-
-
Notifications
You must be signed in to change notification settings - Fork 3
feat: update workflows internal data structure #249
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughThis update introduces new metadata fields ( Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant API
participant WorkflowManager
Client->>API: POST /api/v1/workflow/:id/run { data }
API->>WorkflowManager: getWorkflow(id)
alt Workflow is disabled
API-->>Client: 403 Forbidden (workflow disabled)
else Workflow requires data and data missing
API-->>Client: 400 Bad Request (missing data)
else Valid workflow and data
API->>WorkflowManager: runWorkflow(id, data)
API-->>Client: 202 Accepted { status, result, started, finished }
end
Possibly related PRs
Poem
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/httpServer/swagger/api-v1.yml (1)
401-440: Comprehensive Workflow schema with metadata properties.The Workflow schema properly defines all new metadata fields:
isEnabled/isRequiredAdditionalData: Boolean flags for workflow controloperations: Nullable array for workflow-specific operations with schemasMinor observation: Line 417 defines a
schemaproperty at the workflow level, but based on the implementation, schemas are only defined within theoperationsarray. Consider if this top-levelschemaproperty is needed.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
__tests__/cli/__snapshots__/workflows.test.js.snapis excluded by!**/*.snap
📒 Files selected for processing (4)
__tests__/httpServer/apiV1.test.js(4 hunks)src/cli/workflows.js(2 hunks)src/httpServer/routers/apiV1.js(2 hunks)src/httpServer/swagger/api-v1.yml(3 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/cli/workflows.js (3)
src/workflows/index.js (10)
require(2-2)require(3-3)require(4-4)require(5-5)require(7-7)require(8-8)updateGithubOrgs(10-29)upsertGithubRepositories(31-64)runAllTheComplianceChecks(66-77)upsertOSSFScorecardAnalysis(79-110)src/reports/index.js (1)
generateStaticReports(126-228)src/cli/index.js (1)
workflows(2-2)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Playwright Tests
🔇 Additional comments (12)
src/cli/workflows.js (3)
8-8: LGTM: Schema import added for bulk-import workflow.The import statement correctly loads the JSON schema needed for the bulk-import workflow operations.
10-57: Well-structured metadata addition with consistent implementation.The new metadata properties are consistently added across all workflow definitions:
isRequiredAdditionalData: Properly indicates data requirementsisEnabled: Allows workflows to be disabled (notablyupsert-ossf-scorecardandbulk-import)operations: Only populated forbulk-importwith schema validationThe TODO comment on line 46 appropriately flags the bulk-import workflow for future architectural consideration.
67-74: Metadata properly exposed in workflow details.The
getWorkflowsDetailsfunction correctly includes all new metadata properties in both the workflows object and workflowsList array, ensuring consistent API responses.src/httpServer/routers/apiV1.js (3)
186-186: Correct request body handling for nested data structure.The change from destructuring the entire request body to extracting only the
dataproperty aligns with the new API contract where workflow input is nested under adatafield.
198-204: Robust validation logic with appropriate HTTP status codes.The validation checks are well-implemented:
- Returns 403 (Forbidden) for disabled workflows - semantically correct as the resource exists but is not allowed
- Returns 400 (Bad Request) for missing required data - proper client error for invalid input
The validation order is logical: check enablement first, then data requirements.
210-214: Enhanced response structure with execution timing.The addition of
startedandfinishedtimestamps provides valuable execution metadata. The structured response withstatus,result, and timing information improves API usability and monitoring capabilities.__tests__/httpServer/apiV1.test.js (3)
13-14: Mock objects properly updated with new metadata properties.The mock workflow objects now include the required
isEnabledandisRequiredAdditionalDataproperties, ensuring tests accurately reflect the new workflow structure.Also applies to: 19-19
196-196: Test payload structure aligned with API changes.The change from sending raw data to nesting it under a
dataproperty correctly reflects the updated API contract.
219-263: Comprehensive test coverage for new validation scenarios.Excellent test coverage added for the new validation logic:
- Lines 219-240: Tests disabled workflow returns 403 Forbidden
- Lines 242-263: Tests missing required data returns 400 Bad Request
Both tests properly mock the workflow metadata and verify correct error responses. The test structure follows established patterns and includes appropriate assertions.
src/httpServer/swagger/api-v1.yml (3)
57-57: Improved schema reusability with component references.Replacing inline schemas with component references (
$ref: '#/components/schemas/Workflow'and$ref: '#/components/schemas/WorkflowCompleted') eliminates duplication and improves maintainability of the API specification.Also applies to: 101-101
108-113: Added 403 response for disabled workflows.The new 403 response properly documents the case when a workflow is disabled, matching the validation logic implemented in the API router.
370-400: Well-defined WorkflowCompleted schema.The new component schema accurately models the enhanced workflow execution response:
status: Execution statusresult: Contains message and success flagstarted/finished: ISO datetime timestampsAll properties are appropriately marked as required and include realistic examples.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests