You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@eagle.apache.org by r7raul1984 <gi...@git.apache.org> on 2017/01/09 02:48:46 UTC
[GitHub] eagle pull request #765: [EAGLE-851] Add validation for policy name
GitHub user r7raul1984 opened a pull request:
https://github.com/apache/eagle/pull/765
[EAGLE-851] Add validation for policy name
- Add checkPolicyName method to check the length of policy name.
https://issues.apache.org/jira/browse/EAGLE-851
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/1haodian/eagle EAGLE-851
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/eagle/pull/765.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #765
----
commit 965bb33747e14171833f9970aa1fecc603e37dd3
Author: r7raul1984 <ta...@yhd.com>
Date: 2017-01-09T02:38:21Z
[EAGLE-851] Add validation for policy name
- Add checkPolicyName method to check the length of policy name.
https://issues.apache.org/jira/browse/EAGLE-851
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] eagle pull request #765: [EAGLE-851] Add validation for policy name
Posted by jhsenjaliya <gi...@git.apache.org>.
Github user jhsenjaliya commented on a diff in the pull request:
https://github.com/apache/eagle/pull/765#discussion_r95098391
--- Diff: eagle-server/src/main/webapp/app/dev/public/js/ctrls/alertEditCtrl.js ---
@@ -304,6 +304,15 @@
return false;
};
+
+ $scope.checkPolicyName = function () {
+ if($scope.policy.name.length > 50) {
+ return "length should less than 50";
+ }
+ return false;
--- End diff --
I think it should return true since the name is in valid range at this point.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] eagle issue #765: [EAGLE-851] Add validation for policy name
Posted by jhsenjaliya <gi...@git.apache.org>.
Github user jhsenjaliya commented on the issue:
https://github.com/apache/eagle/pull/765
+1, LGTM
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] eagle pull request #765: [EAGLE-851] Add validation for policy name
Posted by r7raul1984 <gi...@git.apache.org>.
Github user r7raul1984 commented on a diff in the pull request:
https://github.com/apache/eagle/pull/765#discussion_r95098855
--- Diff: eagle-server/src/main/webapp/app/dev/public/js/ctrls/alertEditCtrl.js ---
@@ -304,6 +304,15 @@
return false;
};
+
+ $scope.checkPolicyName = function () {
+ if($scope.policy.name.length > 50) {
+ return "length should less than 50";
+ }
+ return false;
--- End diff --
Yes,I agree with you.But I see checkPublisherName return false. So I consistent with checkPublisherName method.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] eagle issue #765: [EAGLE-851] Add validation for policy name
Posted by haoch <gi...@git.apache.org>.
Github user haoch commented on the issue:
https://github.com/apache/eagle/pull/765
I think the feature should be added into backend `org.apache.eagle.service.metadata.resource.MetadataResource#validatePolicy()`
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] eagle pull request #765: [EAGLE-851] Add validation for policy name
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/eagle/pull/765
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] eagle issue #765: [EAGLE-851] Add validation for policy name
Posted by haoch <gi...@git.apache.org>.
Github user haoch commented on the issue:
https://github.com/apache/eagle/pull/765
But it should be better to add validate in both front-end and backend at same time :-)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] eagle pull request #765: [EAGLE-851] Add validation for policy name
Posted by jhsenjaliya <gi...@git.apache.org>.
Github user jhsenjaliya commented on a diff in the pull request:
https://github.com/apache/eagle/pull/765#discussion_r95098323
--- Diff: eagle-server/src/main/webapp/app/dev/public/js/ctrls/alertEditCtrl.js ---
@@ -304,6 +304,15 @@
return false;
};
+
+ $scope.checkPolicyName = function () {
+ if($scope.policy.name.length > 50) {
+ return "length should less than 50";
--- End diff --
should we say "policy name length should be less than 50" ? just length sounds very generic.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] eagle pull request #765: [EAGLE-851] Add validation for policy name
Posted by r7raul1984 <gi...@git.apache.org>.
Github user r7raul1984 commented on a diff in the pull request:
https://github.com/apache/eagle/pull/765#discussion_r95098715
--- Diff: eagle-server/src/main/webapp/app/dev/public/js/ctrls/alertEditCtrl.js ---
@@ -304,6 +304,15 @@
return false;
};
+
+ $scope.checkPolicyName = function () {
+ if($scope.policy.name.length > 50) {
+ return "length should less than 50";
--- End diff --
This message will combine with "Name". Finally, user will see the message "Name length should less than 50".
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] eagle pull request #765: [EAGLE-851] Add validation for policy name
Posted by jhsenjaliya <gi...@git.apache.org>.
Github user jhsenjaliya commented on a diff in the pull request:
https://github.com/apache/eagle/pull/765#discussion_r95098403
--- Diff: eagle-server/src/main/webapp/app/dev/public/js/ctrls/alertEditCtrl.js ---
@@ -325,6 +334,7 @@
return (
!$scope.saveLock &&
$scope.policy.name &&
+ !$scope.checkPolicyName() &&
--- End diff --
if its changed to return true, should remove !
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] eagle issue #765: [EAGLE-851] Add validation for policy name
Posted by baibaichen <gi...@git.apache.org>.
Github user baibaichen commented on the issue:
https://github.com/apache/eagle/pull/765
+1 on @haoch. By the way, from the perspective of priority, backend validation would be solved at first.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---