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 #7207: Select and display agent mode (verify/enforce) #1236

Conversation

RaphaelGauthier
Copy link
Member

</p>
}
private[this] val policyModes = {
val globalPolicyModeName = configService.rudder_policy_mode_name().getOrElse("") match {
Copy link
Member

Choose a reason for hiding this comment

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

globalPolicyMode should be passed as parameter of DirectiveEditForm

Copy link
Member

Choose a reason for hiding this comment

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

At least somewhere else where you managed a possible error

}
private[this] val policyModes = {
val globalPolicyModeName = configService.rudder_policy_mode_name().getOrElse("") match {
case Verify => "Audit"
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 rename modes to Audit or Enforce, you should do "policyMode.name.capitalize()"

case Enforce => "Enforce"
}
val policyName = directive.policyMode match {
case Some(Verify) => "Audit"
Copy link
Member

Choose a reason for hiding this comment

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

Same here Should use mode.name

@@ -427,6 +430,59 @@ class DirectiveEditForm(
}
s"${version} ${deprecationInfo}"
}
private[this] val globalOverrideText = configService.rudder_policy_overridable().getOrElse("") match {
Copy link
Member

Choose a reason for hiding this comment

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

Same here: globalPolicyMode should be passed as parameter of DirectiveEditForm

@@ -98,13 +101,7 @@ class ReportDisplayer(
def asyncDisplay(node : NodeInfo) : NodeSeq = {
val id = JsNodeId(node.id)
val callback = SHtml.ajaxInvoke(() => SetHtml("reportsDetails",displayReports(node)) )
Script(OnLoad(JsRaw(
Copy link
Member

Choose a reason for hiding this comment

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

Why did you remove the tab "behavior" ?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think I didn't want to do that, my bad... I will check that.

Copy link
Member Author

Choose a reason for hiding this comment

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

I remember now. This code doesn't work since the bind function is deprecated (since Jquery 3.0.0) !

$(nTd).append(editLink);
console.log(oData)
Copy link
Member

Choose a reason for hiding this comment

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

console.log :)

Copy link
Member Author

Choose a reason for hiding this comment

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

Arf :)


app.factory('configGlobalFactory', function ($http){
//Case : global configuration
this.policymode = {
Copy link
Member

Choose a reason for hiding this comment

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

policyMode

});
}
};
this.overridemode = {
Copy link
Member

Choose a reason for hiding this comment

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

overrideMode


app.factory('configNodeFactory', function ($http){
//Case : node configuration
this.policymode = {
Copy link
Member

Choose a reason for hiding this comment

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

policyMode

@@ -273,7 +271,7 @@ function createRuleTable(gridId, data, needCheckbox, needActions, needCompliance
var tooltipId = data.id+"-description";
parent.attr("tooltipid",tooltipId);
parent.attr("title","");
parent.addClass("tooltip tooltipabletr");
parent.addClass("tooltip tooltipabletr");{}
Copy link
Member

Choose a reason for hiding this comment

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

you can remove the {}

$(nTd).append(editLink);
var policyMode = oData.policyMode ? oData.policyMode : "";
Copy link
Member

Choose a reason for hiding this comment

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

Maybe you should keep undefined / treat undefined (find it better to deal with than empty String

Copy link
Member Author

Choose a reason for hiding this comment

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

Right !

var policyName = policyMode == "verify" ? "audit" : policyMode
var span = "<span class='label-text " + labelType + " glyphicon glyphicon-question-sign' data-toggle='tooltip' data-placement='top' data-html='true' title=''></span>"
var badge = $(span).get(0);
var tooltip = "" +
Copy link
Member

Choose a reason for hiding this comment

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

I think the tooltip can be put in an external function ( like : 'policyModetooltip (kind, policyName, explanation)'

var policyMode = currentPolicyMode.toLowerCase();
var nodeOrDirective = isNode ? "node" : "directive";
var labelType = "label-"+policyMode;
var policyName = policyMode == "verify" ? "audit" : policyMode
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 use policyMode everywhere, not policyName

@RaphaelGauthier
Copy link
Member Author

PR updated

@RaphaelGauthier RaphaelGauthier force-pushed the ust_7207/select_and_display_agent_mode_verify_enforce branch from f540106 to 9859b6b Compare October 7, 2016 08:14
@Normation-Quality-Assistant
Copy link
Contributor

This PR is not mergeable to upper versions.
Since it is "Ready for merge" you must merge it by yourself using the following command:
rudder-dev merge https://github.com/Normation/rudder/pull/1236
-- Your faithful QA

@VinceMacBuche
Copy link
Member

OK, merging this PR

@VinceMacBuche VinceMacBuche merged commit 9859b6b into Normation:master Oct 7, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants