-
Notifications
You must be signed in to change notification settings - Fork 0
task: add sort and custom metadata filters to list app runs #48
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
base: main
Are you sure you want to change the base?
Conversation
c3f8290 to
2bee18c
Compare
Codecov Report❌ Patch coverage is
@@ Coverage Diff @@
## main #48 +/- ##
==========================================
+ Coverage 95.59% 95.63% +0.04%
==========================================
Files 9 9
Lines 1157 1191 +34
Branches 158 167 +9
==========================================
+ Hits 1106 1139 +33
- Misses 51 52 +1
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
2bee18c to
3ff4eb6
Compare
3ff4eb6 to
00a3dbb
Compare
|
|
LGTM, however I think we should either add an e2e test now or create a ticket for adding it. |
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.
Pull request overview
This PR adds sorting and custom metadata filtering capabilities to the list-application-runs command in both the SDK and CLI packages. The implementation allows users to filter runs by custom metadata key-value pairs and sort results by various fields.
Key changes:
- Extended SDK
listApplicationRunsmethod withcustomMetadataandsortparameters - Added CLI options for custom metadata filtering (JSON string) and sorting (JSON array of field names)
- Implemented JSON parsing and validation with error handling for both new parameters
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/sdk/src/platform-sdk.ts | Added customMetadata and sort parameters to listApplicationRuns method |
| packages/cli/src/cli.ts | Added command-line options for custom metadata filtering and sorting |
| packages/cli/src/cli-functions.ts | Implemented JSON parsing logic, validation, and error handling for new parameters |
| packages/cli/src/cli-functions.spec.ts | Added comprehensive test coverage for new filtering and sorting features, including error cases |
| ATTRIBUTIONS.md | Updated license formatting for various dependencies (formatting changes only) |
| } | ||
| } | ||
|
|
||
| let sortBy: string[] | undefined = undefined; |
Copilot
AI
Dec 4, 2025
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.
Redundant initialization: let sortBy: string[] | undefined = undefined; can be simplified to let sortBy: string[] | undefined; since variables are already undefined by default.
| let sortBy: string[] | undefined = undefined; | |
| let sortBy: string[] | undefined; |
| }) | ||
| .option('sort', { | ||
| describe: | ||
| 'Sort by field (run_id\napplication_version_id\norganization_id\nstatus\nsubmitted_at\nsubmitted_by\n)', |
Copilot
AI
Dec 4, 2025
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.
The description format with embedded newlines (\n) is unusual for CLI help text and may not render as intended in the help output. Consider using a clearer format like: 'Sort by field (e.g., ["run_id", "-status"])' or provide examples in the description instead of listing all possible fields with newlines.
| 'Sort by field (run_id\napplication_version_id\norganization_id\nstatus\nsubmitted_at\nsubmitted_by\n)', | |
| 'Sort by field (e.g., "run_id", "-status", "submitted_at"). Fields: run_id, application_version_id, organization_id, status, submitted_at, submitted_by.', |
| sort?: string; | ||
| } | ||
| ): Promise<void> { | ||
| console.log('Options received:', options); |
Copilot
AI
Dec 4, 2025
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.
Debug logging statement should be removed before merging to production. This appears to be leftover debugging code that will clutter console output for end users.
| console.log('Options received:', options); |
| await listApplicationRuns('production', mockAuthService, { | ||
| applicationId: 'app1', | ||
| customMetadata: '{"key": "value"}', | ||
| sortBy: '["-created_at"]', |
Copilot
AI
Dec 4, 2025
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.
Incorrect parameter name: sortBy should be sort to match the function signature and other test cases. This test will fail because the listApplicationRuns function expects a parameter named sort, not sortBy.
| sortBy: '["-created_at"]', | |
| sort: '["-created_at"]', |
| console.log('Options received:', options); | ||
| if (options?.customMetadata) { | ||
| try { | ||
| JSON.parse(options?.customMetadata || '{}') as unknown; |
Copilot
AI
Dec 4, 2025
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.
Redundant fallback in JSON.parse(options?.customMetadata || '{}'). Since this code is inside an if (options?.customMetadata) block, the value is guaranteed to be truthy, making the || '{}' fallback unnecessary.
| JSON.parse(options?.customMetadata || '{}') as unknown; | |
| JSON.parse(options.customMetadata) as unknown; |



No description provided.