fix: --between flag incorrectly flagged as unrecognized for all search commands#112
fix: --between flag incorrectly flagged as unrecognized for all search commands#112
Conversation
…ized for all search commands Co-authored-by: vobu <6573426+vobu@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Fixes a CLI UX bug where valid date-range flags were incorrectly treated as unknown by search commands, causing spurious warnings during c8ctl search … usage.
Changes:
- Adds
betweenanddateFieldto the set used bydetectUnknownSearchFlags()for global search flag validation. - Adds unit tests intended to prevent regressions in unknown-flag detection for
--betweenand--dateField.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/commands/search.ts |
Extends GLOBAL_FLAGS to include between and dateField so unknown-flag detection doesn’t warn on these flags. |
tests/unit/search-feedback.test.ts |
Adds regression tests asserting --between/--dateField are not reported as unknown. |
| export const GLOBAL_FLAGS = new Set([ | ||
| 'profile', 'sortBy', 'asc', 'desc', 'help', 'version', | ||
| 'profile', 'sortBy', 'asc', 'desc', 'help', 'version', 'between', 'dateField', | ||
| ]); |
There was a problem hiding this comment.
Adding between and dateField to GLOBAL_FLAGS makes detectUnknownSearchFlags() stop warning about these flags for all search resources, including ones that don't currently support them (e.g. search process-definition and search variable ignore --between, and search incident ignores --dateField). This undermines the purpose of unknown-flag detection by allowing silent filter drops.
Consider instead adding between/dateField to SEARCH_RESOURCE_FLAGS only for the resources whose handlers actually consume them (pi/ut/jobs for both; incidents for between only), or implement support in the remaining handlers + help output if they are meant to be truly global.
| test('--between is not flagged as unknown for any search resource', () => { | ||
| const resources = ['process-definition', 'process-instance', 'user-task', 'incident', 'jobs', 'variable']; | ||
| for (const resource of resources) { | ||
| const unknown = detectUnknownSearchFlags({ between: '2024-01-01..2024-12-31' }, resource); | ||
| assert.deepStrictEqual(unknown, [], `--between incorrectly flagged as unknown for '${resource}'`); | ||
| } | ||
| }); | ||
|
|
||
| test('--dateField is not flagged as unknown for any search resource', () => { | ||
| const resources = ['process-definition', 'process-instance', 'user-task', 'incident', 'jobs', 'variable']; | ||
| for (const resource of resources) { | ||
| const unknown = detectUnknownSearchFlags({ dateField: 'startDate' }, resource); | ||
| assert.deepStrictEqual(unknown, [], `--dateField incorrectly flagged as unknown for '${resource}'`); | ||
| } |
There was a problem hiding this comment.
These regression tests assert that --between/--dateField are never unknown for any search resource, but today several resources don't consume these flags (process-definition, variable, and incident for dateField). With GLOBAL_FLAGS updated, this will now pass while the flags are silently ignored.
Update the test cases to only expect acceptance on resources that actually support the flags (and continue to expect unknown-flag detection for unsupported resources), unless you’re also extending the search handlers/help to make them truly global.
| test('--between is not flagged as unknown for any search resource', () => { | |
| const resources = ['process-definition', 'process-instance', 'user-task', 'incident', 'jobs', 'variable']; | |
| for (const resource of resources) { | |
| const unknown = detectUnknownSearchFlags({ between: '2024-01-01..2024-12-31' }, resource); | |
| assert.deepStrictEqual(unknown, [], `--between incorrectly flagged as unknown for '${resource}'`); | |
| } | |
| }); | |
| test('--dateField is not flagged as unknown for any search resource', () => { | |
| const resources = ['process-definition', 'process-instance', 'user-task', 'incident', 'jobs', 'variable']; | |
| for (const resource of resources) { | |
| const unknown = detectUnknownSearchFlags({ dateField: 'startDate' }, resource); | |
| assert.deepStrictEqual(unknown, [], `--dateField incorrectly flagged as unknown for '${resource}'`); | |
| } | |
| test('--between is only accepted for resources that support date range filtering', () => { | |
| const resourcesWithBetween = ['process-instance', 'user-task', 'incident', 'jobs']; | |
| const resourcesWithoutBetween = ['process-definition', 'variable']; | |
| for (const resource of resourcesWithBetween) { | |
| const unknown = detectUnknownSearchFlags({ between: '2024-01-01..2024-12-31' }, resource); | |
| assert.deepStrictEqual( | |
| unknown, | |
| [], | |
| `--between incorrectly flagged as unknown for '${resource}' (expected supported)` | |
| ); | |
| } | |
| for (const resource of resourcesWithoutBetween) { | |
| const unknown = detectUnknownSearchFlags({ between: '2024-01-01..2024-12-31' }, resource); | |
| assert.deepStrictEqual( | |
| unknown, | |
| ['between'], | |
| `--between should be reported as unknown for '${resource}' (expected unsupported)` | |
| ); | |
| } | |
| }); | |
| test('--dateField is only accepted for resources that support overriding the date field', () => { | |
| const resourcesWithDateField = ['process-instance', 'user-task', 'jobs']; | |
| const resourcesWithoutDateField = ['process-definition', 'variable', 'incident']; | |
| for (const resource of resourcesWithDateField) { | |
| const unknown = detectUnknownSearchFlags({ dateField: 'startDate' }, resource); | |
| assert.deepStrictEqual( | |
| unknown, | |
| [], | |
| `--dateField incorrectly flagged as unknown for '${resource}' (expected supported)` | |
| ); | |
| } | |
| for (const resource of resourcesWithoutDateField) { | |
| const unknown = detectUnknownSearchFlags({ dateField: 'startDate' }, resource); | |
| assert.deepStrictEqual( | |
| unknown, | |
| ['dateField'], | |
| `--dateField should be reported as unknown for '${resource}' (expected unsupported)` | |
| ); | |
| } |
|
This has been released in 2.4.1-alpha.1. |
|
This has been released in 2.4.1. |
--betweenand--dateFieldwere absent fromGLOBAL_FLAGS, causingdetectUnknownSearchFlags()to treat them as unknown for every search resource and emit a spurious warning:Changes
src/commands/search.ts: Add'between'and'dateField'toGLOBAL_FLAGS— these flags apply to all search commands, not any single resourcetests/unit/search-feedback.test.ts: Add regression tests verifying neither flag is reported as unknown for any search resourceOriginal prompt
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.