Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes #8763: When the conditions contain spaces, there are kept and the generated file is invalid #429

Conversation

RaphaelGauthier
Copy link
Member

@@ -257,7 +258,7 @@ <h4 class="panel-title">
<span class="glyphicon glyphicon-plus-sign" style="margin-right:5px"></span>
Add methods
</div>
<button ng-disabled="editForm.$pending || editForm.$invalid || CForm.form.$invalid || checkSelectedTechnique()" class="btn btn-default pull-right" ng-click="saveTechnique()">Save</button>
<button ng-disabled="editForm.$pending || editForm.$invalid || CForm.form.$invalid || checkSelectedTechnique() || selectedMethod.advanced_class.indexOf(' ')>0 || CForm.versionForm.$invalid" class="btn btn-default pull-right" ng-click="saveTechnique()">Save</button>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

YOu should not add the check on wetherit contains a space or not, but be based on the fact that Cform.form is $invalid (check advanced class input with a patter)

<input type="text" name="cfClasses" id="advanced" class="form-control" ng-change="updateClassContext()" ng-model="selectedMethod.advanced_class" placeholder="">
<div class="form-group condition-form" ng-class="{'has-error':selectedMethod.advanced_class.indexOf(' ')>0}">
<label class="control-label" for="advanced">Other CFEngine classes:</label>
<input type="text" class="form-control" name="cfClasses" id="advanced" aria-describedby="helpBlock" ng-change="updateClassContext()" ng-model="selectedMethod.advanced_class" >
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should add a ng-pattern here to check the input ( value should be ng-pattern="/^\S+$/" ) and then you can use angular form validation

<div class="form-group condition-form">
<label for="advanced">Other CFEngine classes:</label>
<input type="text" name="cfClasses" id="advanced" class="form-control" ng-change="updateClassContext()" ng-model="selectedMethod.advanced_class" placeholder="">
<div class="form-group condition-form" ng-class="{'has-error':selectedMethod.advanced_class.indexOf(' ')>0}">
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With angular validation you don't need to have the ng-class

<div class="form-group condition-form" ng-class="{'has-error':selectedMethod.advanced_class.indexOf(' ')>0}">
<label class="control-label" for="advanced">Other CFEngine classes:</label>
<input type="text" class="form-control" name="cfClasses" id="advanced" aria-describedby="helpBlock" ng-change="updateClassContext()" ng-model="selectedMethod.advanced_class" >
<span id="helpBlock" class="help-block" ng-show="selectedMethod.advanced_class.indexOf(' ')>0">
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And you use a ng-message here

@RaphaelGauthier RaphaelGauthier force-pushed the bug_8763/when_the_conditions_contain_spaces_there_are_kept_and_the_generated_file_is_invalid branch from 5a52f4b to 8e39272 Compare September 19, 2016 16:11
@RaphaelGauthier
Copy link
Member Author

PR updated

Copy link
Member

@VinceMacBuche VinceMacBuche left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Greate ! Merging this PR!

@Normation-Quality-Assistant
Copy link
Contributor

OK, merging this PR

@Normation-Quality-Assistant Normation-Quality-Assistant merged commit 8e39272 into Normation:v0.x Sep 21, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants