From 0af1815e63aaabd60bf74d97ec44f53b66d0d023 Mon Sep 17 00:00:00 2001 From: Mann Dharmesh Acharya <89384052+mach-12@users.noreply.github.com> Date: Fri, 22 Mar 2024 18:34:25 +0530 Subject: [PATCH] [GSoC2024] Added Validations, testcases to Labels Constructor (#7627) 1. Added new Label Validation to handle empty label name given to constructor. 2. Fixed existing validation for Duplicate label name to make it more clear. 3. Added tests for both. ### Motivation and context #### Background - Mentioned as an`ui-ux` `enhancement` issue in #5729, Two problems were raised: 1. Hint when user tries to add existing label in project is incorrect as the same code validation is triggered in both project and task. 2. When user enters an empty label name, It is not intuitively clear that the state of the constructor is reset and skeleton is not added. #### Fix 1. Tests were written for the desired outcome. 2. Fixed the Unique Label Validation by changing the display message to a generalized one. Screenshot 2024-03-18 at 1 20 47 AM Screenshot 2024-03-18 at 1 21 03 AM 4. Added a new Empty Label Validation which stops the form from submitting when label name is empty and displays an error message. Screenshot 2024-03-18 at 1 21 20 AM ### How has this been tested? - The issue was first triggered locally. - Test cases were written using Cypress, Here are the screenshots of the same. Screenshot 2024-03-18 at 1 22 29 AM Screenshot 2024-03-18 at 1 23 51 AM ### Checklist - [x] I submit my changes into the `develop` branch - [x] I have created a changelog fragment - [ ] I have updated the documentation accordingly - [x] I have added tests to cover my changes - [x] I have linked related issues (see [GitHub docs]( https://help.github.com/en/github/managing-your-work-on-github/linking-a-pull-request-to-an-issue#linking-a-pull-request-to-an-issue-using-a-keyword)) - [x] I have increased versions of npm packages if it is necessary ([cvat-canvas](https://github.com/opencv/cvat/tree/develop/cvat-canvas#versioning), [cvat-core](https://github.com/opencv/cvat/tree/develop/cvat-core#versioning), [cvat-data](https://github.com/opencv/cvat/tree/develop/cvat-data#versioning) and [cvat-ui](https://github.com/opencv/cvat/tree/develop/cvat-ui#versioning)) ### License - [x] I submit _my code changes_ under the same [MIT License]( https://github.com/opencv/cvat/blob/develop/LICENSE) that covers the project. Feel free to contact the maintainers if that's a concern. --------- Co-authored-by: Kirill Lakhov --- ...ann.compi_labels_constructor_issue_5729.md | 4 +++ .../components/labels-editor/label-form.tsx | 4 +-- ..._118_create_label_with_empty_label_name.js | 27 +++++++++++++++++++ ...change_label_name_via_label_constructor.js | 4 +-- ...3_create_label_with_existing_label_name.js | 2 +- tests/cypress/support/commands.js | 3 +-- 6 files changed, 37 insertions(+), 7 deletions(-) create mode 100644 changelog.d/20240318_195644_mann.compi_labels_constructor_issue_5729.md create mode 100644 tests/cypress/e2e/actions_tasks2/case_118_create_label_with_empty_label_name.js diff --git a/changelog.d/20240318_195644_mann.compi_labels_constructor_issue_5729.md b/changelog.d/20240318_195644_mann.compi_labels_constructor_issue_5729.md new file mode 100644 index 000000000000..bc854e55442f --- /dev/null +++ b/changelog.d/20240318_195644_mann.compi_labels_constructor_issue_5729.md @@ -0,0 +1,4 @@ +### Fixed + +- Label constructor validation of empty label names + () diff --git a/cvat-ui/src/components/labels-editor/label-form.tsx b/cvat-ui/src/components/labels-editor/label-form.tsx index 600dc11af72b..45e9f66bc92d 100644 --- a/cvat-ui/src/components/labels-editor/label-form.tsx +++ b/cvat-ui/src/components/labels-editor/label-form.tsx @@ -493,7 +493,7 @@ export default class LabelForm extends React.Component { name='name' rules={[ { - required: !!label, + required: true, message: 'Please specify a name', }, { @@ -503,7 +503,7 @@ export default class LabelForm extends React.Component { { validator: (_rule: any, labelName: string) => { if (labelNames.includes(labelName) && label?.name !== labelName) { - return Promise.reject(new Error('Label name must be unique for the task')); + return Promise.reject(new Error('Label name must be unique')); } return Promise.resolve(); }, diff --git a/tests/cypress/e2e/actions_tasks2/case_118_create_label_with_empty_label_name.js b/tests/cypress/e2e/actions_tasks2/case_118_create_label_with_empty_label_name.js new file mode 100644 index 000000000000..9c17b0434605 --- /dev/null +++ b/tests/cypress/e2e/actions_tasks2/case_118_create_label_with_empty_label_name.js @@ -0,0 +1,27 @@ +// Copyright (C) 2024 CVAT.ai Corporation +// +// SPDX-License-Identifier: MIT + +/// + +import { taskName } from '../../support/const'; + +context('Creating a label with an empty name.', () => { + const caseId = '118'; + + before(() => { + cy.openTask(taskName); + }); + + describe(`Testing case "${caseId}"`, () => { + it('Should display an error message if label name is empty', () => { + // Attempt to create a label with an empty name + cy.get('.cvat-constructor-viewer-new-item').click(); + cy.get('button[type="submit"]').click(); + + cy.contains('[role="alert"]', 'Please specify a name') + .should('exist') + .and('be.visible'); + }); + }); +}); diff --git a/tests/cypress/e2e/actions_tasks2/case_42_change_label_name_via_label_constructor.js b/tests/cypress/e2e/actions_tasks2/case_42_change_label_name_via_label_constructor.js index 731cf0fdc59c..88d355af28a8 100644 --- a/tests/cypress/e2e/actions_tasks2/case_42_change_label_name_via_label_constructor.js +++ b/tests/cypress/e2e/actions_tasks2/case_42_change_label_name_via_label_constructor.js @@ -21,8 +21,8 @@ context('Changing a label name via label constructor.', () => { it('Set empty label name. Press "Continue" button. Label name is not created. Label constructor is closed.', () => { cy.get('.cvat-constructor-viewer-new-item').click(); // Open label constructor cy.contains('[type="submit"]', 'Continue').click(); - cy.get('.cvat-label-constructor-creator').should('not.exist'); - cy.get('.cvat-constructor-viewer').should('be.visible'); + cy.contains('[role="alert"]', 'Please specify a name').should('exist').and('be.visible'); + cy.contains('[type="button"]', 'Cancel').click(); // Close label constructor }); it('Change label name to any other correct value. Press "Continue" button. The label created.', () => { diff --git a/tests/cypress/e2e/actions_tasks2/case_43_create_label_with_existing_label_name.js b/tests/cypress/e2e/actions_tasks2/case_43_create_label_with_existing_label_name.js index 2cfb60a7aa0e..7454bcb96cc2 100644 --- a/tests/cypress/e2e/actions_tasks2/case_43_create_label_with_existing_label_name.js +++ b/tests/cypress/e2e/actions_tasks2/case_43_create_label_with_existing_label_name.js @@ -24,7 +24,7 @@ context('Creating a label with existing label name.', () => { // Try to create a label with existing label name cy.get('.cvat-constructor-viewer-new-item').click(); cy.get('[placeholder="Label name"]').type(firstLabelName); - cy.contains('[role="alert"]', 'Label name must be unique for the task') // Checking alert visibility + cy.contains('[role="alert"]', 'Label name must be unique') // Checking alert visibility .should('exist') .and('be.visible'); }); diff --git a/tests/cypress/support/commands.js b/tests/cypress/support/commands.js index 385a23c7012e..b5a2b01d9339 100644 --- a/tests/cypress/support/commands.js +++ b/tests/cypress/support/commands.js @@ -1022,8 +1022,7 @@ Cypress.Commands.add('addNewSkeletonLabel', ({ name, points }) => { cy.contains('Continue').scrollIntoView(); cy.contains('Continue').click(); - cy.contains('Continue').scrollIntoView(); - cy.contains('Continue').click(); + cy.contains('Cancel').click(); }); });