feat(work-items): Implement runQuery for Total count output type - #740
feat(work-items): Implement runQuery for Total count output type#740Ahalya-ni wants to merge 5 commits into
Conversation
Signed-off-by: Ahalya Radhakrishnan <ahalya.radhakrishnan@ni.com>
Signed-off-by: Ahalya Radhakrishnan <ahalya.radhakrishnan@ni.com>
There was a problem hiding this comment.
🟡 Changes recommended
The generated combined filter expression has incorrect boolean precedence for multi-type queries, which can return wrong results.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds initial implementation for the Work Items datasource runQuery path to support a “Total Count” output, including backend request helpers and user-facing error handling.
Changes:
- Implemented
runQueryhandling forOutputType.TotalCount(count-only query) and stubbedOutputType.Properties. - Added request/response typing and a work item type → backend filter-value mapping to build query filters.
- Added unit tests covering total-count behavior, filter building, template variable replacement, and error handling.
File summaries
| File | Description |
|---|---|
| src/datasources/work-items/WorkItemsDataSource.ts | Implements total-count querying, filter construction, and custom error handling/alerts. |
| src/datasources/work-items/WorkItemsDataSource.test.ts | Adds tests for the new runQuery behavior and error scenarios. |
| src/datasources/work-items/types.ts | Introduces request/response interfaces used by the new query helpers. |
| src/datasources/work-items/constants.ts | Adds a mapping from UI type options to backend type filter values. |
Review details
- Files reviewed: 4/4 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Signed-off-by: Ahalya Radhakrishnan <ahalya.radhakrishnan@ni.com>
There was a problem hiding this comment.
🟡 Changes recommended
The new default error-message path can produce unhelpful messages when the backend error lacks a parsed inner message, and the included fix should be applied before merge.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (2)
Previously missed (1) — in code that hasn't changed since the last review.
src/datasources/work-items/WorkItemsDataSource.ts:112
- In the default error-message branch,
extractErrorInfocan return an emptymessage(e.g., when the thrown error contains a status code but noError message:segment). That produces a user-facing message like(status 500) .which is not actionable; also the catch assumeserroris always anErrorwith amessageproperty.
src/datasources/work-items/WorkItemsDataSource.test.ts:120
- This test name refers to a "queryBy" filter, but the WorkItems query field is named
filter. Renaming the description makes it clear which query input is being templated.
it('should replace template variables in the queryBy filter', async () => {
- Files reviewed: 4/4 changed files
- Comments generated: 1
- Review effort level: Lite
Signed-off-by: Ahalya Radhakrishnan <ahalya.radhakrishnan@ni.com>
There was a problem hiding this comment.
🔵 Needs a closer look
The current filter-combining logic can change boolean operator precedence, potentially returning incorrect counts for complex filters, and the tests should be updated accordingly.
Review details
Suppressed comments (3)
Previously missed (1) — in code that hasn't changed since the last review.
src/datasources/work-items/WorkItemsDataSource.ts:112
- In the default error case,
errorDetails.messagecan be empty when the thrown error doesn’t include anError message:segment (e.g., only a status code). That would produce a user-facing message like(status 500) .with no details; add a fallback string when no parsed message is available.
src/datasources/work-items/WorkItemsDataSource.ts:130
- When combining two filters with
&&, operator precedence can change the meaning if either side contains||(e.g.,A && B || C). To preserve the original semantics of each filter expression, wrap each filter in parentheses when both are present (and treat whitespace-only filters as empty).
protected buildQueryFilter(filterA?: string, filterB?: string): string | undefined {
const filters = [filterA, filterB].filter(Boolean);
return filters.length > 0 ? filters.join(' && ') : undefined;
}
src/datasources/work-items/WorkItemsDataSource.test.ts:71
- This expectation should match the datasource's combined filter formatting once both filters are parenthesized to preserve operator precedence.
expect(postSpy).toHaveBeenCalledWith(
'/niworkitem/v1/query-workitems',
{
filter: '(type = "workorder" || type = "testplan") && state = "NEW"',
take: 0,
- Files reviewed: 4/4 changed files
- Comments generated: 0 new
- Review effort level: Lite
Signed-off-by: Ahalya Radhakrishnan <ahalya.radhakrishnan@ni.com>
Pull Request
🤨 Rationale
This PR enable the Work Items datasource to query and display the total number of matching work items.
This provides the first supported
runQueryoutput type while preserving an empty response forProperties output (It will be implemented in upcoming PRs).
👩💻 Implementation
runQuerysupport forOutputType.TotalCount.OutputType.Propertiesreturning an empty data frame.🧪 Testing
Added unit tests
✅ Checklist