> On April 26, 2016, 5:46 p.m., Andrii Tkach wrote:
> > ambari-web/app/controllers/main/service/widgets/create/step3_controller.js, 
> > line 34
> > <https://reviews.apache.org/r/46475/diff/3/?file=1359898#file1359898line34>
> >
> >     Default value should be boolean, not string.

Hello Andrii,
I have updated this in the new patch "AMBARI-15979-April-26.patch"

Thank you


> On April 26, 2016, 5:46 p.m., Andrii Tkach wrote:
> > ambari-web/app/controllers/main/service/widgets/create/step3_controller.js, 
> > line 74
> > <https://reviews.apache.org/r/46475/diff/3/?file=1359898#file1359898line74>
> >
> >     Default value should be boolean, not string.

I have updated this in the new patch "AMBARI-15979-April-26.patch"


> On April 26, 2016, 5:46 p.m., Andrii Tkach wrote:
> > ambari-web/app/controllers/main/service/widgets/create/step3_controller.js, 
> > line 115
> > <https://reviews.apache.org/r/46475/diff/3/?file=1359898#file1359898line115>
> >
> >     Please put validations in utils/validation.js

I have updated this in the new patch "AMBARI-15979-April-26.patch"


> On April 26, 2016, 5:46 p.m., Andrii Tkach wrote:
> > ambari-web/app/controllers/main/service/widgets/create/step3_controller.js, 
> > line 134
> > <https://reviews.apache.org/r/46475/diff/3/?file=1359898#file1359898line134>
> >
> >     Please put validations in utils/validation.js

I have updated this in the new patch "AMBARI-15979-April-26.patch"


- Keta


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46475/#review130653
-----------------------------------------------------------


On April 26, 2016, 6:45 p.m., Keta Patel wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46475/
> -----------------------------------------------------------
> 
> (Updated April 26, 2016, 6:45 p.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 8c8b9e5 
>   ambari-web/app/templates/main/service/widgets/create/step3.hbs 9f431af 
>   ambari-web/app/utils/validator.js 490fec5 
>   
> 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
> 
> 
> File Attachments
> ----------------
> 
> AMBARI-15979-inlineError.patch
>   
> https://reviews.apache.org/media/uploaded/files/2016/04/21/03bc972a-b076-4520-948b-3a204082eca0__AMBARI-15979-inlineError.patch
> AMBARI-15979-topError.patch
>   
> https://reviews.apache.org/media/uploaded/files/2016/04/21/8b4e9a7b-96cb-4a17-9c9c-56e59b5fd349__AMBARI-15979-topError.patch
> 
> 
> Thanks,
> 
> Keta Patel
> 
>

Reply via email to