----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46475/#review129904 -----------------------------------------------------------
ambari-web/app/templates/main/service/widgets/create/step3.hbs (line 24) <https://reviews.apache.org/r/46475/#comment193453> Why are these styled as warnings? Shouldn't they be errors? ambari-web/app/templates/main/service/widgets/create/step3.hbs <https://reviews.apache.org/r/46475/#comment193454> Can you restore inline error display instead of the single warning display at the top of the page ? ambari-web/app/templates/main/service/widgets/create/step3.hbs <https://reviews.apache.org/r/46475/#comment193455> Can you restore inline error display instead of the single warning display at the top of the page ? - Di Li On April 21, 2016, 5:47 a.m., Keta Patel wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/46475/ > ----------------------------------------------------------- > > (Updated April 21, 2016, 5:47 a.m.) > > > Review request for Ambari, Andrii Tkach and Di Li. > > > Bugs: AMBARI-15979 > https://issues.apache.org/jira/browse/AMBARI-15979 > > > Repository: ambari > > > Description > ------- > > ISSUE: > The UI validation at present checks only the length of the user input for > widget_name and description fields. All characters are allowed to be stored > in the database through them. A more strict UI validation that limits the > type of characters entered for these fields will provide a good first line of > defense. > > Steps to reproduce: > 1. Make sure you have Ambari Metrics service installed on your cluster. > 2. On the Dashboard, select any service that makes use of Ambari Metrics, say > HDFS. > 3. In the "Metrics" section, click the "Actions" button in the top-right > corner, and select "Create a new widget" option from the drop-down. > (attachment: create_widget_button_location.tiff) > 4. Create widget pop-up is displayed. > 5. On Step-1, select any type for the widget and click "Next". (attachment: > create_widget_step1.tiff) > 6. On Step-2, select any valid metrics parameter and click "Next". > (attachment: create_widget_step2.tiff) > 7. On Step-3, for widget_name and description fields, you can enter any > character. No validation is present to check the contents. The only > validation present checks the length of the input text. > (attachments: > create_widget_step3.tiff, > original_characters_allowed_for_name_and_description.tiff, > original_length_validation_for_name.tiff, > original_length_validation_for_description.tiff ) > > > FIX: > The UI validation is enhanced by checking the content of the input for name > and description. > The patch attached allows only alphanumeric, underscore, hyphen, space and > percentage symbol to be valid characters for both fields. > The % symbol is added as part of the white-list as there are existing widgets > that contain % symbols in their names. In order to keep the characters in the > name consistent, this was decided. > The description could probably have a little more flexibility in terms of > characters allowed. Any suggestions to update this validation would be > helpful. > The warning message template is also updated to conform to the existing norms > used in several Ambari pop-ups (e.g. Manage Config Groups, Manage Alert > Groups). > > The images attached (starting with "fixed_") show how the fix appears with > the patch. > > > Diffs > ----- > > ambari-web/app/controllers/main/service/widgets/create/step3_controller.js > dd7a93f > ambari-web/app/messages.js 6c0049d > ambari-web/app/templates/main/service/widgets/create/step3.hbs 9f431af > > ambari-web/test/controllers/main/service/widgets/create/step3_controller_test.js > 6f92142 > > Diff: https://reviews.apache.org/r/46475/diff/ > > > Testing > ------- > > Tests are added that check the validate functions added for widget name and > description. > Both widget name and description are tested for: > 1. all valid characters > 2. invalid characters > 3. length of input > 4. empty string > > Ambari-Web tests with the patch: > 25671 tests complete (33 seconds) > 154 tests pending > > > Thanks, > > Keta Patel > >
