Skip to content

Commit f967820

Browse files
miloszmarcinkowskipaulinashakirova
miloszmarcinkowski
authored and
paulinashakirova
committed
[APM][Agent configuration] Prevent users from attempting to save invalid settings (elastic#232206)
Closes elastic#231651 Follow up of elastic#230086 This PR improves validation on the Agent configuration page by: - Disabling the Save button when predefined settings contain validation errors, - Filtering out settings with empty keys before calling the API (to prevent API errors and loss of unsaved changes), - Triggering validation on submit to prevent users from attempting to save new advanced settings with either key or value empty. Demo: https://github.com/user-attachments/assets/c55da3e2-1a3e-4604-ac27-40165a2752cc
1 parent 7a1885d commit f967820

6 files changed

Lines changed: 262 additions & 178 deletions

File tree

‎x-pack/solutions/observability/plugins/apm/ftr_e2e/cypress/e2e/settings/agent_configurations.cy.ts‎

Lines changed: 91 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -175,7 +175,7 @@ describe('Agent configuration', () => {
175175
cy.contains('Send traces');
176176
cy.contains('Send metrics');
177177
cy.contains('Send logs');
178-
cy.get('[data-test-subj="row_deactivate_all_instrumentations"')
178+
cy.getByTestSubj('row_deactivate_all_instrumentations')
179179
.find('[data-test-subj="apmSelectWithPlaceholderSelect"]')
180180
.select('true');
181181
cy.contains('Save configuration').click();
@@ -185,7 +185,7 @@ describe('Agent configuration', () => {
185185
cy.get('[data-test-subj="apmColumnsButton"]').then((e) => {
186186
e[2].click();
187187
});
188-
cy.get('[data-test-subj="confirmModalConfirmButton"]').click();
188+
cy.getByTestSubj('confirmModalConfirmButton').click();
189189
cy.contains('No configurations found');
190190
});
191191

@@ -212,8 +212,8 @@ describe('Agent configuration', () => {
212212
.click();
213213

214214
cy.contains('Next step').click();
215-
cy.get('[data-test-subj="settingsPage_serviceName"]').contains('All');
216-
cy.get('[data-test-subj="settingsPage_environmentName"]').contains('All');
215+
cy.getByTestSubj('settingsPage_serviceName').contains('All');
216+
cy.getByTestSubj('settingsPage_environmentName').contains('All');
217217
cy.contains('Edit').click();
218218
cy.wait('@serviceEnvironmentApi');
219219
cy.getByTestSubj('serviceEnviromentComboBox')
@@ -249,54 +249,44 @@ describe('Agent configuration', () => {
249249
cy.contains('Advanced configuration allows you to define custom settings').should(
250250
'be.visible'
251251
);
252-
cy.get('[data-test-subj="apmSettingsAddAdvancedConfigurationButton"]').should('be.visible');
253-
cy.get('[data-test-subj="apmAdvancedConfigurationDocumentationLink"]').should('be.visible');
252+
cy.getByTestSubj('apmSettingsAddAdvancedConfigurationButton').should('be.visible');
253+
cy.getByTestSubj('apmAdvancedConfigurationDocumentationLink').should('be.visible');
254254
});
255255

256256
it('should add a new advanced configuration row when clicking add button', () => {
257-
cy.get('[data-test-subj="apmSettingsAdvancedConfigurationKeyField"]').should('not.exist');
258-
cy.get('[data-test-subj="apmSettingsAdvancedConfigurationValueField"]').should('not.exist');
257+
cy.getByTestSubj('apmSettingsAdvancedConfigurationKeyField').should('not.exist');
258+
cy.getByTestSubj('apmSettingsAdvancedConfigurationValueField').should('not.exist');
259259

260-
cy.get('[data-test-subj="apmSettingsAddAdvancedConfigurationButton"]').click();
260+
cy.getByTestSubj('apmSettingsAddAdvancedConfigurationButton').click();
261261

262-
cy.get('[data-test-subj="apmSettingsAdvancedConfigurationKeyField"]').should('be.visible');
263-
cy.get('[data-test-subj="apmSettingsAdvancedConfigurationValueField"]').should(
264-
'be.visible'
265-
);
266-
cy.get('[data-test-subj="apmSettingsRemoveAdvancedConfigurationButton"]').should(
267-
'be.visible'
268-
);
262+
cy.getByTestSubj('apmSettingsAdvancedConfigurationKeyField').should('be.visible');
263+
cy.getByTestSubj('apmSettingsAdvancedConfigurationValueField').should('be.visible');
264+
cy.getByTestSubj('apmSettingsRemoveAdvancedConfigurationButton').should('be.visible');
269265
});
270266

271267
it('should remove advanced configuration row when clicking remove button', () => {
272-
cy.get('[data-test-subj="apmSettingsAddAdvancedConfigurationButton"]').click();
273-
cy.get('[data-test-subj="apmSettingsAdvancedConfigurationKeyField"]').should(
274-
'have.length',
275-
1
276-
);
268+
cy.getByTestSubj('apmSettingsAddAdvancedConfigurationButton').click();
269+
cy.getByTestSubj('apmSettingsAdvancedConfigurationKeyField').should('have.length', 1);
277270

278-
cy.get('[data-test-subj="apmSettingsRemoveAdvancedConfigurationButton"]').first().click();
279-
cy.get('[data-test-subj="apmSettingsAdvancedConfigurationKeyField"]').should(
280-
'have.length',
281-
0
282-
);
271+
cy.getByTestSubj('apmSettingsRemoveAdvancedConfigurationButton').first().click();
272+
cy.getByTestSubj('apmSettingsAdvancedConfigurationKeyField').should('have.length', 0);
283273
});
284274

285275
describe('Key Input Validation', () => {
286276
beforeEach(() => {
287-
cy.get('[data-test-subj="apmSettingsAddAdvancedConfigurationButton"]').click();
277+
cy.getByTestSubj('apmSettingsAddAdvancedConfigurationButton').click();
288278
});
289279

290280
it('should show error when key is empty after interaction', () => {
291-
cy.get('[data-test-subj="apmSettingsAdvancedConfigurationKeyField"]')
281+
cy.getByTestSubj('apmSettingsAdvancedConfigurationKeyField')
292282
.first()
293283
.type('custom.key')
294284
.clear();
295285
cy.contains('Key cannot be empty').should('be.visible');
296286
});
297287

298288
it('should show error when key conflicts with predefined settings', () => {
299-
cy.get('[data-test-subj="apmSettingsAdvancedConfigurationKeyField"]')
289+
cy.getByTestSubj('apmSettingsAdvancedConfigurationKeyField')
300290
.first()
301291
.type('logging_level');
302292

@@ -306,18 +296,12 @@ describe('Agent configuration', () => {
306296
});
307297

308298
it('should show error when duplicate keys are entered', () => {
309-
cy.get('[data-test-subj="apmSettingsAddAdvancedConfigurationButton"]').click();
310-
311299
const duplicateKey = 'custom.setting.duplicate';
312-
cy.get('[data-test-subj="apmSettingsAdvancedConfigurationKeyField"]')
313-
.first()
314-
.type(duplicateKey);
300+
cy.getByTestSubj('apmSettingsAdvancedConfigurationKeyField').first().type(duplicateKey);
315301

316-
cy.get('[data-test-subj="apmSettingsAddAdvancedConfigurationButton"]').click();
302+
cy.getByTestSubj('apmSettingsAddAdvancedConfigurationButton').click();
317303

318-
cy.get('[data-test-subj="apmSettingsAdvancedConfigurationKeyField"]')
319-
.first()
320-
.type(duplicateKey);
304+
cy.getByTestSubj('apmSettingsAdvancedConfigurationKeyField').first().type(duplicateKey);
321305

322306
cy.contains('This key is already used in another advanced configuration').should(
323307
'be.visible'
@@ -326,9 +310,7 @@ describe('Agent configuration', () => {
326310

327311
it('should accept valid custom keys', () => {
328312
const validKey = 'custom.my.setting';
329-
cy.get('[data-test-subj="apmSettingsAdvancedConfigurationKeyField"]')
330-
.first()
331-
.type(validKey);
313+
cy.getByTestSubj('apmSettingsAdvancedConfigurationKeyField').first().type(validKey);
332314

333315
cy.contains('Key cannot be empty').should('not.exist');
334316
cy.contains('This key is already predefined').should('not.exist');
@@ -338,11 +320,11 @@ describe('Agent configuration', () => {
338320

339321
describe('Value Input Validation', () => {
340322
beforeEach(() => {
341-
cy.get('[data-test-subj="apmSettingsAddAdvancedConfigurationButton"]').click();
323+
cy.getByTestSubj('apmSettingsAddAdvancedConfigurationButton').click();
342324
});
343325

344326
it('should show error when value is empty after interaction', () => {
345-
cy.get('[data-test-subj="apmSettingsAdvancedConfigurationValueField"]')
327+
cy.getByTestSubj('apmSettingsAdvancedConfigurationValueField')
346328
.first()
347329
.type('custom.value')
348330
.clear();
@@ -351,14 +333,61 @@ describe('Agent configuration', () => {
351333

352334
it('should accept valid values', () => {
353335
const validValue = 'true';
354-
cy.get('[data-test-subj="apmSettingsAdvancedConfigurationValueField"]')
355-
.first()
356-
.type(validValue);
336+
cy.getByTestSubj('apmSettingsAdvancedConfigurationValueField').first().type(validValue);
357337

358338
cy.contains('Value cannot be empty').should('not.exist');
359339
});
360340
});
361341

342+
describe('Adding New Row Validation', () => {
343+
beforeEach(() => {
344+
// change predefined setting so action bar with save button is active
345+
cy.getByTestSubj('row_deactivate_all_instrumentations')
346+
.find('[data-test-subj="apmSelectWithPlaceholderSelect"]')
347+
.select('true');
348+
349+
// trigger validation errors
350+
cy.getByTestSubj('apmSettingsAddAdvancedConfigurationButton').click();
351+
cy.contains('Save configuration').click();
352+
353+
cy.contains('Value cannot be empty').should('be.visible');
354+
cy.contains('Key cannot be empty').should('be.visible');
355+
});
356+
357+
it('should show validation errors and remove them after providing valid values', () => {
358+
const customKey = 'custom.test.setting';
359+
const customValue = 'test-value';
360+
361+
cy.getByTestSubj('apmSettingsAdvancedConfigurationKeyField').first().type(customKey);
362+
cy.getByTestSubj('apmSettingsAdvancedConfigurationValueField').first().type(customValue);
363+
364+
cy.contains('Value cannot be empty').should('not.exist');
365+
cy.contains('Key cannot be empty').should('not.exist');
366+
});
367+
368+
it('should show validation errors and remove them after discarding changes', () => {
369+
// discard changes
370+
cy.getByTestSubj('apmBottomBarActionsDiscardChangesButton').click();
371+
372+
// reopen new config row
373+
cy.getByTestSubj('apmSettingsAddAdvancedConfigurationButton').click();
374+
375+
cy.contains('Value cannot be empty').should('not.exist');
376+
cy.contains('Key cannot be empty').should('not.exist');
377+
});
378+
379+
it('should show validation errors and remove them after deleting row', () => {
380+
// delete row
381+
cy.getByTestSubj('apmSettingsRemoveAdvancedConfigurationButton').click();
382+
383+
// reopen new config row
384+
cy.getByTestSubj('apmSettingsAddAdvancedConfigurationButton').click();
385+
386+
cy.contains('Value cannot be empty').should('not.exist');
387+
cy.contains('Key cannot be empty').should('not.exist');
388+
});
389+
});
390+
362391
describe('Complete Configuration Flow', () => {
363392
// delete congfiguration after test, otherwise unable to create new one
364393
afterEach(() => {
@@ -368,23 +397,19 @@ describe('Agent configuration', () => {
368397
cy.get('[data-test-subj="apmColumnsButton"]').then((e) => {
369398
e[2].click();
370399
});
371-
cy.get('[data-test-subj="confirmModalConfirmButton"]').click();
400+
cy.getByTestSubj('confirmModalConfirmButton').click();
372401
cy.contains('No configurations found');
373402
});
374403

375404
it('should save configuration with advanced settings', () => {
376-
cy.get('[data-test-subj="apmSettingsAddAdvancedConfigurationButton"]').click();
405+
cy.getByTestSubj('apmSettingsAddAdvancedConfigurationButton').click();
377406

378407
const customKey = 'custom.test.setting';
379408
const customValue = 'test-value';
380409

381-
cy.get('[data-test-subj="apmSettingsAdvancedConfigurationKeyField"]')
382-
.first()
383-
.type(customKey);
410+
cy.getByTestSubj('apmSettingsAdvancedConfigurationKeyField').first().type(customKey);
384411

385-
cy.get('[data-test-subj="apmSettingsAdvancedConfigurationValueField"]')
386-
.first()
387-
.type(customValue);
412+
cy.getByTestSubj('apmSettingsAdvancedConfigurationValueField').first().type(customValue);
388413

389414
cy.contains('Save configuration').click();
390415

@@ -394,21 +419,20 @@ describe('Agent configuration', () => {
394419
});
395420

396421
it('should save configuration with multiple advanced settings', () => {
397-
cy.get('[data-test-subj="apmSettingsAddAdvancedConfigurationButton"]').click();
398-
cy.get('[data-test-subj="apmSettingsAdvancedConfigurationKeyField"]')
422+
cy.getByTestSubj('apmSettingsAddAdvancedConfigurationButton').click();
423+
cy.getByTestSubj('apmSettingsAdvancedConfigurationKeyField')
399424
.first()
400425
.type('custom.setting.one');
401-
cy.get('[data-test-subj="apmSettingsAdvancedConfigurationValueField"]')
426+
cy.getByTestSubj('apmSettingsAdvancedConfigurationValueField')
402427
.first()
428+
.type('value-one')
403429
.type('value-one');
404430

405-
cy.get('[data-test-subj="apmSettingsAddAdvancedConfigurationButton"]').click();
406-
cy.get('[data-test-subj="apmSettingsAdvancedConfigurationKeyField"]')
431+
cy.getByTestSubj('apmSettingsAddAdvancedConfigurationButton').click();
432+
cy.getByTestSubj('apmSettingsAdvancedConfigurationKeyField')
407433
.first()
408434
.type('custom.setting.two');
409-
cy.get('[data-test-subj="apmSettingsAdvancedConfigurationValueField"]')
410-
.first()
411-
.type('value-two');
435+
cy.getByTestSubj('apmSettingsAdvancedConfigurationValueField').first().type('value-two');
412436

413437
cy.contains('Save configuration').click();
414438

@@ -419,14 +443,14 @@ describe('Agent configuration', () => {
419443

420444
describe('Documentation Link', () => {
421445
it('should have correct documentation link for Java agent', () => {
422-
cy.get('[data-test-subj="apmAdvancedConfigurationDocumentationLink"]')
446+
cy.getByTestSubj('apmAdvancedConfigurationDocumentationLink')
423447
.should('have.attr', 'href')
424448
.and(
425449
'include',
426450
'https://www.elastic.co/docs/reference/opentelemetry/edot-sdks/java/configuration'
427451
);
428452

429-
cy.get('[data-test-subj="apmAdvancedConfigurationDocumentationLink"]').should(
453+
cy.getByTestSubj('apmAdvancedConfigurationDocumentationLink').should(
430454
'have.attr',
431455
'target',
432456
'_blank'
@@ -436,17 +460,13 @@ describe('Agent configuration', () => {
436460

437461
describe('Edge Cases and Error Handling', () => {
438462
it('should handle special characters in keys and values', () => {
439-
cy.get('[data-test-subj="apmSettingsAddAdvancedConfigurationButton"]').click();
463+
cy.getByTestSubj('apmSettingsAddAdvancedConfigurationButton').click();
440464

441465
const specialKey = 'custom.key-with_special.chars123';
442466
const specialValue = 'value-with-special@chars#123!';
443467

444-
cy.get('[data-test-subj="apmSettingsAdvancedConfigurationKeyField"]')
445-
.first()
446-
.type(specialKey);
447-
cy.get('[data-test-subj="apmSettingsAdvancedConfigurationValueField"]')
448-
.first()
449-
.type(specialValue);
468+
cy.getByTestSubj('apmSettingsAdvancedConfigurationKeyField').first().type(specialKey);
469+
cy.getByTestSubj('apmSettingsAdvancedConfigurationValueField').first().type(specialValue);
450470

451471
cy.contains('Key cannot be empty').should('not.exist');
452472
cy.contains('Value cannot be empty').should('not.exist');

0 commit comments

Comments
 (0)