Skip to content

Commit d917a46

Browse files
CopilotCopilot
authored
fix: address unresolved review comments — empty-string guards, SQL OR parentheses, normalizeQueryText nullish check
Agent-Logs-Url: https://github.com/ydb-platform/ydb-embedded-ui/sessions/08189414-41d3-403e-b9ad-56181f13ca78
1 parent a2e3df2 commit d917a46

6 files changed

Lines changed: 95 additions & 13 deletions

File tree

‎src/containers/Tenant/Diagnostics/TopQueries/__test__/utils.test.ts‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,34 +68,64 @@ describe('createQueryInfoItems', () => {
6868
expect(wmItem?.content).toBe(0);
6969
});
7070

71+
it('should skip WmPoolId when empty string', () => {
72+
const result = createQueryInfoItems({WmPoolId: ''});
73+
const wmItem = result.find((item) => item.name === 'WM Pool ID');
74+
expect(wmItem).toBeUndefined();
75+
});
76+
7177
it('should include WmState when present', () => {
7278
const result = createQueryInfoItems({WmState: 'Running'});
7379
const wmItem = result.find((item) => item.name === 'WM State');
7480
expect(wmItem).toBeDefined();
7581
expect(wmItem?.content).toBe('Running');
7682
});
7783

84+
it('should skip WmState when empty string', () => {
85+
const result = createQueryInfoItems({WmState: ''});
86+
const wmItem = result.find((item) => item.name === 'WM State');
87+
expect(wmItem).toBeUndefined();
88+
});
89+
7890
it('should include WmEnterTime when present', () => {
7991
const result = createQueryInfoItems({WmEnterTime: '2024-01-01T00:00:00Z'});
8092
const wmItem = result.find((item) => item.name === 'WM Enter Time');
8193
expect(wmItem).toBeDefined();
8294
expect(wmItem?.content).toBeTruthy();
8395
});
8496

97+
it('should skip WmEnterTime when empty string', () => {
98+
const result = createQueryInfoItems({WmEnterTime: ''});
99+
const wmItem = result.find((item) => item.name === 'WM Enter Time');
100+
expect(wmItem).toBeUndefined();
101+
});
102+
85103
it('should include WmExitTime when present', () => {
86104
const result = createQueryInfoItems({WmExitTime: '2024-01-01T00:00:00Z'});
87105
const wmItem = result.find((item) => item.name === 'WM Exit Time');
88106
expect(wmItem).toBeDefined();
89107
expect(wmItem?.content).toBeTruthy();
90108
});
91109

110+
it('should skip WmExitTime when empty string', () => {
111+
const result = createQueryInfoItems({WmExitTime: ''});
112+
const wmItem = result.find((item) => item.name === 'WM Exit Time');
113+
expect(wmItem).toBeUndefined();
114+
});
115+
92116
it('should include ClientAddress when present', () => {
93117
const result = createQueryInfoItems({ClientAddress: '192.168.1.1'});
94118
const item = result.find((item) => item.name === 'Client Address');
95119
expect(item).toBeDefined();
96120
expect(item?.content).toBe('192.168.1.1');
97121
});
98122

123+
it('should skip ClientAddress when empty string', () => {
124+
const result = createQueryInfoItems({ClientAddress: ''});
125+
const item = result.find((item) => item.name === 'Client Address');
126+
expect(item).toBeUndefined();
127+
});
128+
99129
it('should include ClientPID with falsy value 0', () => {
100130
const result = createQueryInfoItems({ClientPID: 0});
101131
const item = result.find((item) => item.name === 'Client PID');
@@ -110,13 +140,25 @@ describe('createQueryInfoItems', () => {
110140
expect(item?.content).toBe('ydb-go-sdk/3.0');
111141
});
112142

143+
it('should skip ClientUserAgent when empty string', () => {
144+
const result = createQueryInfoItems({ClientUserAgent: ''});
145+
const item = result.find((item) => item.name === 'Client User Agent');
146+
expect(item).toBeUndefined();
147+
});
148+
113149
it('should include ClientSdkBuildInfo when present', () => {
114150
const result = createQueryInfoItems({ClientSdkBuildInfo: 'v1.2.3'});
115151
const item = result.find((item) => item.name === 'Client SDK Build Info');
116152
expect(item).toBeDefined();
117153
expect(item?.content).toBe('v1.2.3');
118154
});
119155

156+
it('should skip ClientSdkBuildInfo when empty string', () => {
157+
const result = createQueryInfoItems({ClientSdkBuildInfo: ''});
158+
const item = result.find((item) => item.name === 'Client SDK Build Info');
159+
expect(item).toBeUndefined();
160+
});
161+
120162
it('should include all fields when all are present', () => {
121163
const data: KeyValueRow = {
122164
QueryText: 'SELECT 1',

‎src/containers/Tenant/Diagnostics/TopQueries/columns/columns.tsx‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,7 @@ const wmPoolIdColumn: Column<KeyValueRow> = {
132132
name: QUERIES_COLUMNS_IDS.WmPoolId,
133133
header: QUERIES_COLUMNS_TITLES.WmPoolId,
134134
render: ({row}) => (
135-
<div className={b('text-cell')}>{row.WmPoolId ?? EMPTY_DATA_PLACEHOLDER}</div>
135+
<div className={b('text-cell')}>{row.WmPoolId || EMPTY_DATA_PLACEHOLDER}</div>
136136
),
137137
width: 150,
138138
};
@@ -141,7 +141,7 @@ const wmStateColumn: Column<KeyValueRow> = {
141141
name: QUERIES_COLUMNS_IDS.WmState,
142142
header: QUERIES_COLUMNS_TITLES.WmState,
143143
render: ({row}) => (
144-
<div className={b('text-cell')}>{row.WmState ?? EMPTY_DATA_PLACEHOLDER}</div>
144+
<div className={b('text-cell')}>{row.WmState || EMPTY_DATA_PLACEHOLDER}</div>
145145
),
146146
width: 120,
147147
};
@@ -150,7 +150,7 @@ const wmEnterTimeColumn: Column<KeyValueRow> = {
150150
name: QUERIES_COLUMNS_IDS.WmEnterTime,
151151
header: QUERIES_COLUMNS_TITLES.WmEnterTime,
152152
render: ({row}) =>
153-
row.WmEnterTime !== null && row.WmEnterTime !== undefined
153+
row.WmEnterTime !== null && row.WmEnterTime !== undefined && row.WmEnterTime !== ''
154154
? formatDateTime(new Date(row.WmEnterTime as string).getTime())
155155
: EMPTY_DATA_PLACEHOLDER,
156156
width: 200,
@@ -160,7 +160,7 @@ const wmExitTimeColumn: Column<KeyValueRow> = {
160160
name: QUERIES_COLUMNS_IDS.WmExitTime,
161161
header: QUERIES_COLUMNS_TITLES.WmExitTime,
162162
render: ({row}) =>
163-
row.WmExitTime !== null && row.WmExitTime !== undefined
163+
row.WmExitTime !== null && row.WmExitTime !== undefined && row.WmExitTime !== ''
164164
? formatDateTime(new Date(row.WmExitTime as string).getTime())
165165
: EMPTY_DATA_PLACEHOLDER,
166166
width: 200,

‎src/containers/Tenant/Diagnostics/TopQueries/utils.ts‎

Lines changed: 19 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -93,35 +93,39 @@ export function createQueryInfoItems(data: KeyValueRow): YDBDefinitionListItem[]
9393
});
9494
}
9595

96-
if (data.WmPoolId !== null && data.WmPoolId !== undefined) {
96+
if (data.WmPoolId !== null && data.WmPoolId !== undefined && data.WmPoolId !== '') {
9797
items.push({
9898
name: columnsI18n('wm-pool-id'),
9999
content: data.WmPoolId,
100100
});
101101
}
102102

103-
if (data.WmState !== null && data.WmState !== undefined) {
103+
if (data.WmState !== null && data.WmState !== undefined && data.WmState !== '') {
104104
items.push({
105105
name: columnsI18n('wm-state'),
106106
content: data.WmState,
107107
});
108108
}
109109

110-
if (data.WmEnterTime !== null && data.WmEnterTime !== undefined) {
110+
if (data.WmEnterTime !== null && data.WmEnterTime !== undefined && data.WmEnterTime !== '') {
111111
items.push({
112112
name: columnsI18n('wm-enter-time'),
113113
content: formatDateTime(new Date(data.WmEnterTime as string).getTime()),
114114
});
115115
}
116116

117-
if (data.WmExitTime !== null && data.WmExitTime !== undefined) {
117+
if (data.WmExitTime !== null && data.WmExitTime !== undefined && data.WmExitTime !== '') {
118118
items.push({
119119
name: columnsI18n('wm-exit-time'),
120120
content: formatDateTime(new Date(data.WmExitTime as string).getTime()),
121121
});
122122
}
123123

124-
if (data.ClientAddress !== null && data.ClientAddress !== undefined) {
124+
if (
125+
data.ClientAddress !== null &&
126+
data.ClientAddress !== undefined &&
127+
data.ClientAddress !== ''
128+
) {
125129
items.push({
126130
name: columnsI18n('client-address'),
127131
content: data.ClientAddress,
@@ -135,14 +139,22 @@ export function createQueryInfoItems(data: KeyValueRow): YDBDefinitionListItem[]
135139
});
136140
}
137141

138-
if (data.ClientUserAgent !== null && data.ClientUserAgent !== undefined) {
142+
if (
143+
data.ClientUserAgent !== null &&
144+
data.ClientUserAgent !== undefined &&
145+
data.ClientUserAgent !== ''
146+
) {
139147
items.push({
140148
name: columnsI18n('client-user-agent'),
141149
content: data.ClientUserAgent,
142150
});
143151
}
144152

145-
if (data.ClientSdkBuildInfo !== null && data.ClientSdkBuildInfo !== undefined) {
153+
if (
154+
data.ClientSdkBuildInfo !== null &&
155+
data.ClientSdkBuildInfo !== undefined &&
156+
data.ClientSdkBuildInfo !== ''
157+
) {
146158
items.push({
147159
name: columnsI18n('client-sdk-build-info'),
148160
content: data.ClientSdkBuildInfo,

‎src/store/reducers/executeTopQueries/__test__/utils.test.ts‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,34 @@ describe('normalizeQueryResult', () => {
4040
});
4141
});
4242

43+
it('should remap when QueryText is null', () => {
44+
const input: IQueryResult = {
45+
resultSets: [
46+
{
47+
result: [{Query: 'SELECT 1', QueryText: null}],
48+
},
49+
],
50+
};
51+
52+
const result = normalizeQueryResult(input);
53+
54+
expect(result.resultSets?.[0]?.result?.[0]).toEqual({QueryText: 'SELECT 1'});
55+
});
56+
57+
it('should remap when QueryText is empty string', () => {
58+
const input: IQueryResult = {
59+
resultSets: [
60+
{
61+
result: [{Query: 'SELECT 1', QueryText: ''}],
62+
},
63+
],
64+
};
65+
66+
const result = normalizeQueryResult(input);
67+
68+
expect(result.resultSets?.[0]?.result?.[0]).toEqual({QueryText: 'SELECT 1'});
69+
});
70+
4371
it('should handle rows without Query field', () => {
4472
const input: IQueryResult = {
4573
resultSets: [

‎src/store/reducers/executeTopQueries/executeTopQueries.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ function getRunningQueriesText(
8181
limit?: number,
8282
) {
8383
const filterConditions = filters?.text
84-
? `Query ILIKE '%${filters.text}%' OR UserSID ILIKE '%${filters.text}%'`
84+
? `(Query ILIKE '%${filters.text}%' OR UserSID ILIKE '%${filters.text}%')`
8585
: '';
8686

8787
const orderBy = prepareOrderByFromTableSort(sortOrder);

‎src/store/reducers/executeTopQueries/utils.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ export function getFiltersConditions(tableName: string, filters?: TopQueriesFilt
5353
* SELECT * returns 'Query', but the UI uses 'QueryText'.
5454
*/
5555
function normalizeQueryToQueryText(row: KeyValueRow): KeyValueRow {
56-
if (row.Query !== undefined && row.QueryText === undefined) {
56+
if (row.Query !== undefined && !row.QueryText) {
5757
const {Query, ...rest} = row;
5858
return {...rest, QueryText: Query};
5959
}

0 commit comments

Comments
 (0)