feat(work-items): add query editor for SystemLink Work Items data source - #735
feat(work-items): add query editor for SystemLink Work Items data source#735shivanshu-ni wants to merge 28 commits into
Conversation
…and update authentication URL
…fault query values and validation
There was a problem hiding this comment.
Pull request overview
This PR enhances the Work Items Grafana datasource/query editor by introducing explicit default query values and UI-side validation so queries start in a predictable state and invalid inputs are constrained before being used.
Changes:
- Added strongly-typed query fields (output type, work item types, order-by, sort direction, take limit) plus shared constants/messages.
- Implemented datasource-side default query initialization + normalization (
prepareQuery,normalizeTypes,normalizeTake). - Replaced the query editor placeholder with real controls (output/type/order/descending/take) and added/updated unit tests.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/datasources/work-items/WorkItemsDataSource.ts | Adds concrete defaults and query normalization via prepareQuery. |
| src/datasources/work-items/WorkItemsDataSource.test.ts | Adds unit tests to assert defaulting and normalization behavior. |
| src/datasources/work-items/types.ts | Introduces enums/options/constants for the new query editor + datasource behavior. |
| src/datasources/work-items/components/WorkItemsQueryEditor.tsx | Implements the full query editor UI and validation logic. |
| src/datasources/work-items/components/WorkItemsQueryEditor.test.tsx | Updates tests to reflect the new UI and initialization behavior. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
… querying work items
…p query editor logic
… example.yaml and README
…b.com/ni/systemlink-grafana-plugins into users/shivanshu/feat/workitem-query-editor
…nstants for take limit and tooltips
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.
Suppressed comments (2)
src/datasources/work-items/components/WorkItemsQueryEditor.tsx:61
- When
takeis invalid, the editor still callsonChangewith the invalid numeric value (e.g.,NaNor negative) and only suppressesonRunQueryfor that one interaction. Because the invalid value is now in the persisted query state, a subsequent change (like toggling Output Type) will triggeronRunQuerywith that invalidtakestill present.
To align with the PR goal of preventing invalid query states, avoid writing invalid take values into the query model (keep a separate draft input state and only commit to query.take when valid).
const onTakeChange = (event: React.FormEvent<HTMLInputElement>) => {
const value = parseInt((event.target as HTMLInputElement).value, 10);
if (Number.isNaN(value) || value < 0) {
setTakeInvalidMessage(takeErrorMessages.greaterOrEqualToZero);
handleQueryChange({ ...query, take: value }, false);
src/datasources/work-items/WorkItemsDataSource.ts:48
prepareQueryuses nullish coalescing fortake, which means invalid numeric values likeNaNor negative numbers (both possible from UI input parsing) will bypass the default and be preserved in the prepared query. This can lead to invalid backend requests oncerunQuerystarts usingtake.
Consider normalizing take to the default when it’s not a finite non-negative number.
take: prepared.take ?? this.defaultQuery.take,
…n query editor and data source
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.
Suppressed comments (5)
src/datasources/work-items/constants/QueryEditor.constants.ts:13
- The
taketooltip text is hard-coded to 10,000, which can drift ifTAKE_LIMITchanges. UseTAKE_LIMITin the tooltip string to keep it consistent.
take: 'Set the maximum number of work items to return. Maximum is 10,000.',
src/datasources/work-items/WorkItemsDataSource.ts:62
normalizeTakecurrently accepts any finite non-negative number, which allows values above the editor’s documented limit (10,000). This can reintroduce invalid query states (e.g., via JSON edits) and potentially cause backend errors. Consider enforcing the same upper bound at the datasource layer.
normalizeTake(take?: number): number {
return Number.isFinite(take) && (take as number) >= 0 ? (take as number) : this.defaultQuery.take;
}
src/datasources/work-items/types.ts:5
- Empty options interfaces in this repo are typically declared on a single line (e.g.,
DataFrameDataSourceOptions). Keeping this one-line avoids unnecessary diffs and stays consistent with existing style.
export interface WorkItemsDataSourceOptions extends DataSourceJsonData {
}
src/datasources/work-items/constants/QueryEditor.constants.ts:6
takeErrorMessages.lessOrEqualToTenThousandis hard-coded to 10,000, which can drift ifTAKE_LIMITchanges. Derive the message fromTAKE_LIMITto keep validation text consistent with the actual limit.
This issue also appears on line 13 of the same file.
export const takeErrorMessages = {
greaterOrEqualToZero: 'Enter a value greater than or equal to 0',
lessOrEqualToTenThousand: 'Enter a value less than or equal to 10,000',
};
src/datasources/work-items/WorkItemsDataSource.test.ts:28
- This test asserts that
takevalues above the UI/documented max (10,000) are preserved. If the datasource enforces the same limit as the query editor, the test should use an in-range value (or assert normalization behavior for out-of-range inputs).
take: 12000,
});
expect(query.types).toEqual([WorkItemTypeOptions.All]);
expect(query.take).toBe(12000);
…a source with limits and improved validation
…b.com/ni/systemlink-grafana-plugins into users/shivanshu/feat/workitem-query-editor
|
The Query By field and that two-column layout come with the query builder story — it isn't part of this PR. Right now all controls stack vertically since there's no builder to sit beside them. I'll align the layout with this design in the query builder PR. |
…ry builder integration.
346fc24 to
f8c782e
Compare
feat(work-items): add query editor for SystemLink Work Items data source
feat(work-items): add query editor for SystemLink Work Items data source…ant and clean up unused code in query editor
| <Stack | ||
| direction="column" | ||
| > | ||
| <Stack direction="column" gap={0}> | ||
| <InlineField | ||
| label={labels.outputType} | ||
| labelWidth={LABEL_WIDTH} | ||
| tooltip={tooltips.outputType} | ||
| > | ||
| <RadioButtonGroup | ||
| options={outputTypeOptions} | ||
| onChange={onOutputTypeChange} | ||
| value={query.outputType} | ||
| /> | ||
| </InlineField> | ||
| <InlineField | ||
| label={labels.types} | ||
| labelWidth={LABEL_WIDTH} | ||
| tooltip={tooltips.types} | ||
| invalid={!isTypesValid} | ||
| error={typesErrorMessages.atLeastOneRequired} | ||
| > | ||
| <MultiCombobox | ||
| placeholder={placeholders.types} | ||
| options={WorkItemTypes} | ||
| value={query.types} | ||
| onChange={onTypesChange} | ||
| enableAllOption | ||
| width="auto" | ||
| minWidth={CONTROL_WIDTH} | ||
| maxWidth={CONTROL_WIDTH} | ||
| /> | ||
| </InlineField> | ||
| </Stack> | ||
| {query.outputType === OutputType.Properties && ( | ||
| <InlineField | ||
| label={labels.properties} | ||
| labelWidth={LABEL_WIDTH} | ||
| tooltip={tooltips.properties} | ||
| invalid={!isPropertiesValid} | ||
| error={propertiesErrorMessages.atLeastOneRequired} | ||
| > | ||
| <MultiCombobox | ||
| placeholder={placeholders.properties} | ||
| options={propertiesOptions} | ||
| value={query.properties} | ||
| onChange={onPropertiesChange} | ||
| width="auto" | ||
| minWidth={CONTROL_WIDTH} | ||
| maxWidth={CONTROL_WIDTH} | ||
| /> | ||
| </InlineField> | ||
| )} | ||
| <Stack> | ||
| <InlineField | ||
| label={labels.queryBy} | ||
| labelWidth={LABEL_WIDTH} | ||
| tooltip={tooltips.filter} | ||
| > | ||
| <WorkItemsQueryBuilder /> | ||
| </InlineField> | ||
| {query.outputType === OutputType.Properties && ( | ||
| <Stack direction="column" gap={1}> | ||
| <Stack direction="column" gap={0}> | ||
| <InlineField | ||
| label={labels.orderBy} | ||
| labelWidth={LABEL_WIDTH} | ||
| tooltip={tooltips.orderBy} | ||
| > | ||
| <Combobox | ||
| options={OrderBy} | ||
| placeholder={placeholders.orderBy} | ||
| onChange={onOrderByChange} | ||
| value={query.orderBy} | ||
| width={COMBOBOX_WIDTH} | ||
| /> | ||
| </InlineField> | ||
| <InlineField | ||
| label={labels.descending} | ||
| labelWidth={LABEL_WIDTH} | ||
| tooltip={tooltips.descending} | ||
| > | ||
| <InlineSwitch | ||
| onChange={event => onDescendingChange(event.currentTarget.checked)} | ||
| value={query.descending} | ||
| /> | ||
| </InlineField> | ||
| </Stack> | ||
| <InlineField | ||
| label={labels.take} | ||
| labelWidth={LABEL_WIDTH} | ||
| tooltip={tooltips.take} | ||
| invalid={!!takeInvalidMessage} | ||
| error={takeInvalidMessage} | ||
| > | ||
| <AutoSizeInput | ||
| minWidth={COMBOBOX_WIDTH} | ||
| maxWidth={COMBOBOX_WIDTH} | ||
| type="number" | ||
| value={query.take} | ||
| onBlur={onTakeChange} | ||
| placeholder={placeholders.take} | ||
| onKeyDown={event => { | ||
| validateNumericInput(event); | ||
| }} | ||
| /> | ||
| </InlineField> | ||
| </Stack> | ||
| )} | ||
| </Stack> | ||
| </Stack> |
There was a problem hiding this comment.
| <Stack | |
| direction="column" | |
| > | |
| <Stack direction="column" gap={0}> | |
| <InlineField | |
| label={labels.outputType} | |
| labelWidth={LABEL_WIDTH} | |
| tooltip={tooltips.outputType} | |
| > | |
| <RadioButtonGroup | |
| options={outputTypeOptions} | |
| onChange={onOutputTypeChange} | |
| value={query.outputType} | |
| /> | |
| </InlineField> | |
| <InlineField | |
| label={labels.types} | |
| labelWidth={LABEL_WIDTH} | |
| tooltip={tooltips.types} | |
| invalid={!isTypesValid} | |
| error={typesErrorMessages.atLeastOneRequired} | |
| > | |
| <MultiCombobox | |
| placeholder={placeholders.types} | |
| options={WorkItemTypes} | |
| value={query.types} | |
| onChange={onTypesChange} | |
| enableAllOption | |
| width="auto" | |
| minWidth={CONTROL_WIDTH} | |
| maxWidth={CONTROL_WIDTH} | |
| /> | |
| </InlineField> | |
| </Stack> | |
| {query.outputType === OutputType.Properties && ( | |
| <InlineField | |
| label={labels.properties} | |
| labelWidth={LABEL_WIDTH} | |
| tooltip={tooltips.properties} | |
| invalid={!isPropertiesValid} | |
| error={propertiesErrorMessages.atLeastOneRequired} | |
| > | |
| <MultiCombobox | |
| placeholder={placeholders.properties} | |
| options={propertiesOptions} | |
| value={query.properties} | |
| onChange={onPropertiesChange} | |
| width="auto" | |
| minWidth={CONTROL_WIDTH} | |
| maxWidth={CONTROL_WIDTH} | |
| /> | |
| </InlineField> | |
| )} | |
| <Stack> | |
| <InlineField | |
| label={labels.queryBy} | |
| labelWidth={LABEL_WIDTH} | |
| tooltip={tooltips.filter} | |
| > | |
| <WorkItemsQueryBuilder /> | |
| </InlineField> | |
| {query.outputType === OutputType.Properties && ( | |
| <Stack direction="column" gap={1}> | |
| <Stack direction="column" gap={0}> | |
| <InlineField | |
| label={labels.orderBy} | |
| labelWidth={LABEL_WIDTH} | |
| tooltip={tooltips.orderBy} | |
| > | |
| <Combobox | |
| options={OrderBy} | |
| placeholder={placeholders.orderBy} | |
| onChange={onOrderByChange} | |
| value={query.orderBy} | |
| width={COMBOBOX_WIDTH} | |
| /> | |
| </InlineField> | |
| <InlineField | |
| label={labels.descending} | |
| labelWidth={LABEL_WIDTH} | |
| tooltip={tooltips.descending} | |
| > | |
| <InlineSwitch | |
| onChange={event => onDescendingChange(event.currentTarget.checked)} | |
| value={query.descending} | |
| /> | |
| </InlineField> | |
| </Stack> | |
| <InlineField | |
| label={labels.take} | |
| labelWidth={LABEL_WIDTH} | |
| tooltip={tooltips.take} | |
| invalid={!!takeInvalidMessage} | |
| error={takeInvalidMessage} | |
| > | |
| <AutoSizeInput | |
| minWidth={COMBOBOX_WIDTH} | |
| maxWidth={COMBOBOX_WIDTH} | |
| type="number" | |
| value={query.take} | |
| onBlur={onTakeChange} | |
| placeholder={placeholders.take} | |
| onKeyDown={event => { | |
| validateNumericInput(event); | |
| }} | |
| /> | |
| </InlineField> | |
| </Stack> | |
| )} | |
| </Stack> | |
| </Stack> | |
| <Stack direction="column" gap={0}> | |
| <InlineField | |
| label={labels.outputType} | |
| labelWidth={LABEL_WIDTH} | |
| tooltip={tooltips.outputType} | |
| > | |
| <RadioButtonGroup | |
| options={outputTypeOptions} | |
| onChange={onOutputTypeChange} | |
| value={query.outputType} | |
| /> | |
| </InlineField> | |
| <InlineField | |
| label={labels.types} | |
| labelWidth={LABEL_WIDTH} | |
| tooltip={tooltips.types} | |
| invalid={!isTypesValid} | |
| error={typesErrorMessages.atLeastOneRequired} | |
| > | |
| <MultiCombobox | |
| placeholder={placeholders.types} | |
| options={WorkItemTypes} | |
| value={query.types} | |
| onChange={onTypesChange} | |
| enableAllOption | |
| width="auto" | |
| minWidth={CONTROL_WIDTH} | |
| maxWidth={CONTROL_WIDTH} | |
| /> | |
| </InlineField> | |
| {query.outputType === OutputType.Properties && ( | |
| <> | |
| <Space v={1} /> | |
| <InlineField | |
| label={labels.properties} | |
| labelWidth={LABEL_WIDTH} | |
| tooltip={tooltips.properties} | |
| invalid={!isPropertiesValid} | |
| error={propertiesErrorMessages.atLeastOneRequired} | |
| > | |
| <MultiCombobox | |
| placeholder={placeholders.properties} | |
| options={propertiesOptions} | |
| value={query.properties} | |
| onChange={onPropertiesChange} | |
| width="auto" | |
| minWidth={CONTROL_WIDTH} | |
| maxWidth={CONTROL_WIDTH} | |
| /> | |
| </InlineField> | |
| </> | |
| )} | |
| <Space v={1} /> | |
| <Stack> | |
| <InlineField | |
| label={labels.queryBy} | |
| labelWidth={LABEL_WIDTH} | |
| tooltip={tooltips.filter} | |
| > | |
| <WorkItemsQueryBuilder /> | |
| </InlineField> | |
| {query.outputType === OutputType.Properties && ( | |
| <Stack direction="column" gap={0}> | |
| <InlineField | |
| label={labels.orderBy} | |
| labelWidth={LABEL_WIDTH} | |
| tooltip={tooltips.orderBy} | |
| > | |
| <Combobox | |
| options={OrderBy} | |
| placeholder={placeholders.orderBy} | |
| onChange={onOrderByChange} | |
| value={query.orderBy} | |
| width={COMBOBOX_WIDTH} | |
| /> | |
| </InlineField> | |
| <InlineField | |
| label={labels.descending} | |
| labelWidth={LABEL_WIDTH} | |
| tooltip={tooltips.descending} | |
| > | |
| <InlineSwitch | |
| onChange={event => onDescendingChange(event.currentTarget.checked)} | |
| value={query.descending} | |
| /> | |
| </InlineField> | |
| <Space v={1} /> | |
| <InlineField | |
| label={labels.take} | |
| labelWidth={LABEL_WIDTH} | |
| tooltip={tooltips.take} | |
| invalid={!!takeInvalidMessage} | |
| error={takeInvalidMessage} | |
| > | |
| <AutoSizeInput | |
| minWidth={COMBOBOX_WIDTH} | |
| maxWidth={COMBOBOX_WIDTH} | |
| type="number" | |
| value={query.take} | |
| onBlur={onTakeChange} | |
| placeholder={placeholders.take} | |
| onKeyDown={event => { | |
| validateNumericInput(event); | |
| }} | |
| /> | |
| </InlineField> | |
| </Stack> | |
| )} | |
| </Stack> | |
| </Stack> |
Lets use the space component to introduce the space between controls - this should reduce the number of stacks being used
| export const takeErrorMessages = { | ||
| greaterOrEqualToZero: 'Enter a value greater than or equal to 0', | ||
| lessOrEqualToTenThousand: `Enter a value less than or equal to ${TAKE_LIMIT.toLocaleString()}`, | ||
| }; |
There was a problem hiding this comment.
Create a task to get this User visible strings with the PO - same goes with all the info text on each control
|
|
||
| const onTakeChange = (event: React.FormEvent<HTMLInputElement>) => { | ||
| const value = parseInt((event.target as HTMLInputElement).value, 10); | ||
| if (Number.isNaN(value) || value <= 0) { |
| return; | ||
| } | ||
|
|
||
| if (value > TAKE_LIMIT) { |
| setTakeInvalidMessage(takeErrorMessages.greaterOrEqualToZero); | ||
| return; |
There was a problem hiding this comment.
Do we have test case to check the invalid message?
| render({}); | ||
|
|
||
| expect(screen.getByText('Work item ID')).toBeTruthy(); | ||
| expect(screen.getAllByText(labels.properties)[1]).toBeTruthy(); |
There was a problem hiding this comment.
Why don't we check if all the properties are present and only Work item ID?
There was a problem hiding this comment.
All 5 default properties are already verified at the data layer in WorkItemsDataSource.test.ts ('applies expected default query values'), which asserts query.properties equals the full default array (ID, NAME, TYPE, STATE, WORKSPACE) directly from prepareQuery.
This component test only smoke-checks that the editor renders the default selection, since asserting all 5 as individual DOM tags isn't reliable here — MultiCombobox collapses extra selections into a "+N" overflow badge under jsdom (no real layout is computed), so only the first tag is actually present in the DOM.
We follow the same pattern in the alarms editor (ListAlarmsQueryEditor.test.tsx) — it also never asserts multiple simultaneously-selected MultiCombobox tags, checking a single rendered property label at the component level and validating multi-value state via the handleQueryChange callback/data layer instead.
|
|
||
| render({}); | ||
|
|
||
| expect(screen.getByText('Work item ID')).toBeTruthy(); |
There was a problem hiding this comment.
I guess we can move all these kinds of selectors (screen.getByText) into a page object to keep the test file clean.
Apply wherever applicable
| }; | ||
|
|
||
| // TODO: AB#3923375 - Dummy Query By scaffolding for the query editor PR | ||
| globalVariableOptions = (): QueryBuilderOption[] => this.getVariableOptions(); |
There was a problem hiding this comment.
Why are we introducing this in this PR ? where we have not used it anywhere


Pull Request
🤨 Rationale
This PR improves Work Items query editing by adding safer default values and stronger validation.
It prevents invalid query states and makes the editor behavior more predictable.
👩💻 Implementation
🧪 Testing
✅ Checklist