You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@trafficcontrol.apache.org by GitBox <gi...@apache.org> on 2019/02/23 03:05:39 UTC

[GitHub] ocket8888 opened a new pull request #3361: Added pattern validation for truly valid xmlids

ocket8888 opened a new pull request #3361: Added pattern validation for truly valid xmlids
URL: https://github.com/apache/trafficcontrol/pull/3361
 
 
   ## Which issue is fixed by this PR? If not related to an existing issue, what does this PR do?
   This PR partially addresses Issue #3312 by performing front-end validation on the XML_ID field of new, changed, or requested Delivery Services. Specifically, the validation checks for a whole-field match against the regular expression: `[a-z0-9][a-z0-9\-]*`. Input that doesn't match this *may still be submitted via the API and stored by the database*. The XML_ID is not always used as part of an <abbr title="Fully Qualified Domain Name">FQDN</abbr>, so such constraints on the server-side are better left until such a time that the future of the XML_ID - regarding whether it should be semantically distinct from a DNS label - is decided.
   
   This PR also includes other quality-of-life improvements for Delivery Service forms.
   
   * All `<label>`s now have an appropriate `for` attribute indicating which `<input>` (or `<select>`) it is actually labeling
   * Changed many `ng-pattern` attributes to `pattern`, `ng-maxlength` attributes to `maxlength`.
   * Form fields that allow the selection from among alternate `<options>` having a default, initial value of `Select...` will no longer include that value in the list of selectable options.
   * Added a warning about hazy compatibility when attempting to set a new or existing Delivery Service's `geoProvider` to "Neustar".
   * Fields that use enumerated names like e.g. `qstringIgnore` now have the integral value obscured, containing instead a human-readable description of the selected behavior. (They still pass the appropriate, integral value back to the API)
   * Fields that previously allowed selecting either "True" or "False" now have more context-sensitive names - this removes a dependency on enumerating a hard-coded array (`[{"label": "false", "value": false}, {"label": "true", "value": true}]`) for every true/false field in the form. (Incoming PR to change those to checkboxes for sheer simplicity and effective communication of the represented idea)
   * Fields that defined numbers and relied on `ng-pattern="/^\d+$/"` to ensure only natural numbers were entered now rely instead on the `min` and `step` attributes
   * Latitude and Longitude fields now validate that the entered number is within the allowed range for each ([-90,90] and [-180,180], respectively)
   * Fields that contained URLs have been updated to have `type="url"`
   * `initalDispersion` is now an `<input type="number" min="1" max="10" step="1"/>` instead of a `<select>` iterating over a hard-coded Javascript array of those numbers.
   * "More" menu delimeters are now `<hr/>`s instead of `<li class="divider"></li>`, as they are not actually items in the list but rather horizontal rules that separate them. (Incoming PR to change that `<ul>` to a `<menu>` and do the styling without classes on the `<hr/>` tags)
   
   ## Which TC components are affected by this PR?
   
   - [ ] Documentation
   - [ ] Grove
   - [ ] Traffic Analytics
   - [ ] Traffic Monitor
   - [ ] Traffic Ops
   - [ ] Traffic Ops ORT
   - [x] Traffic Portal
   - [ ] Traffic Router
   - [ ] Traffic Stats
   - [ ] Traffic Vault
   - [ ] Other _________
   
   ## What is the best way to verify this PR? Please include manual steps or automated tests. 
   Run the Traffic Portal UI tests, build and run Traffic Portal and check out the Delivery Service creation, modification and request forms.
   
   
   ## Check all that apply
   
   - [ ] This PR includes tests
   - [ ] This PR includes documentation updates
   - [ ] This PR includes an update to CHANGELOG.md
   - [ ] This PR includes all required license headers
   - [ ] This PR includes a database migration (ensure that migration sequence is correct)
   - [ ] This PR fixes a serious security flaw. Read more: [www.apache.org/security](http://www.apache.org/security/)

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services