On 05/11/2016 03:28 PM, Petr Vobornik wrote:
On 03/31/2016 04:59 PM, Pavel Vomacka wrote:
Hello,

This patch adds option to add host dialog which allows to show generated
OTP.
The patch also changes the way of informing user about success of adding
host
but only when the 'Generate OTP' option is checked.

https://fedorahosted.org/freeipa/ticket/4602

The patch copy&pastes behavior of entity adder dialog buttons when the
purpose is to do additional stuff on success. IMHO it copies to much logic.

Also the following method of redefining add handler is not very object
oriented:
   that.get_button('add_and_edit').click = function() {

Wouldn't it be better to move anonymous success handlers in
entity_adder_dialog to a class methods to achieve it. E.g:

"""
     click: function() {
         that.hide_message();
         that.add(
             function(data, text_status, xhr) {
                 that.added.notify([data], that);
                 that.close();
                 var result = data.result.result;
                 that.show_edit_page(that.entity, result);
                 that.notify_success(data);
             },
             that.on_error);
     }
"""
to
"""
     click: function() {
         that.hide_message();
         that.add(that.on_add_success, that.on_error);
     }

that.on_add_success = function(data, text_status, xhr) {
     that.added.notify([data], that);
     that.close();
     var result = data.result.result;
     that.show_edit_page(that.entity, result);
     that.notify_success(data);
};

that.entity_adder_dialog_on_add_success = that.on_add_success;

"""

so in child class it would be overriden e.g. by:

that.on_add_success = function(data, text_status, xhr) {
     that.entity_adder_dialog_on_add_success(data, text_status, xhr);
     // .. my new code
};

It follows the pattern as in other code.

Other possible emthod is to implement in parent class
handle_notifications override point and then change calls of
that.notify_success(data); to that.handle_notifications(data, method);
Which could be overridden in child.

Or probably my favorite:
entity_adder_dialog has 'added' event which is raised prior closing the
dialog (in 'add' and 'add and edit'). We could either register new event
handler which would to the stuff. It will need a way to distinguish
buttons. The button name/method could be added as addional param in the
base class:
    that.added.notify([data, 'add'], that);

Or a new event could be created if it is important to call it after
dialog is closed.
   that.post_added = IPA.observer();
   that.post_added.notify([data, 'add'], that);

dialog.post_added.attach(function(data, method) {
    // do something;
});


Thank you for awesome explanation of how it should be done. I've chosen the last solution which you described. I added another parameter to the 'added' event and I also added init method which allows to register listener to 'added' event only once. Edited patch is attached.

Pavel^3
From 6167b883e03a550af6833b94d0187e9a35d6372c Mon Sep 17 00:00:00 2001
From: Pavel Vomacka <pvoma...@redhat.com>
Date: Wed, 30 Mar 2016 10:19:39 +0200
Subject: [PATCH] Add option to show OTP when adding host

Add option to add host dialog which allows to show generated OTP.
This patch also changed the way of informing user about success of adding host
but only when the 'Generate OTP' option is checked. There is a new dialog with
generated OTP.

https://fedorahosted.org/freeipa/ticket/4602
---
 install/ui/src/freeipa/add.js      |  6 ++--
 install/ui/src/freeipa/host.js     | 60 ++++++++++++++++++++++++++++++++++++++
 install/ui/test/data/ipa_init.json |  3 ++
 ipalib/plugins/internal.py         |  3 ++
 4 files changed, 69 insertions(+), 3 deletions(-)

diff --git a/install/ui/src/freeipa/add.js b/install/ui/src/freeipa/add.js
index 8f24c7733d1614aaf05b544ecfb641ff57f292f2..6221085269b86d5bbc53c9deec182e10491452ca 100644
--- a/install/ui/src/freeipa/add.js
+++ b/install/ui/src/freeipa/add.js
@@ -84,7 +84,7 @@ IPA.entity_adder_dialog = function(spec) {
                 that.hide_message();
                 that.add(
                     function(data, text_status, xhr) {
-                        that.added.notify([data], that);
+                        that.added.notify([data, 'add_and_add_another'], that);
                         that.show_message(that.get_success_message(data), 'success');
                         that.reset();
                         that.focus_first_element();
@@ -100,7 +100,7 @@ IPA.entity_adder_dialog = function(spec) {
                 that.hide_message();
                 that.add(
                     function(data, text_status, xhr) {
-                        that.added.notify([data], that);
+                        that.added.notify([data, 'add_and_edit'], that);
                         that.close();
                         var result = data.result.result;
                         that.show_edit_page(that.entity, result);
@@ -129,7 +129,7 @@ IPA.entity_adder_dialog = function(spec) {
         that.hide_message();
         that.add(
             function(data, text_status, xhr) {
-                that.added.notify([data], that);
+                that.added.notify([data, 'add'], that);
                 that.close();
                 that.notify_success(data);
             },
diff --git a/install/ui/src/freeipa/host.js b/install/ui/src/freeipa/host.js
index 764e551b40a00d3a35ea4e8ec99de9164bc97be3..817e62efca7dca9745c71ee4d4d4823f885a8fab 100644
--- a/install/ui/src/freeipa/host.js
+++ b/install/ui/src/freeipa/host.js
@@ -347,6 +347,12 @@ return {
                         $type: 'force_host_add_checkbox',
                         name: 'force',
                         metadata: '@mc-opt:host_add:force'
+                    },
+                    {
+                        $type: 'checkbox',
+                        name: 'random',
+                        label: '@i18n:objects.host.generate_otp',
+                        title: '@i18n:objects.host.generate_otp'
                     }
                 ]
             }
@@ -529,13 +535,67 @@ IPA.host_adder_dialog = function(spec) {
 
     var that = IPA.entity_adder_dialog(spec);
 
+    that.init = function() {
+        that.added.attach(that.handle_random_otp);
+    };
+
     that.create_content = function() {
         that.entity_adder_dialog_create_content();
         that.container.addClass('host-adder-dialog');
     };
 
+    that.create_result_dialog = function() {
+        spec = {
+            title: text.get('@i18n:dialogs.result'),
+            sections: [
+                {
+                    show_header: false,
+                    fields: [
+                        {
+                            $type: 'text',
+                            name: 'randompassword',
+                            label: '@i18n:objects.host.generated_otp',
+                            read_only: true
+                        }
+                    ]
+                }
+            ]
+        };
+
+        var dialog = IPA.dialog(spec);
+
+        dialog.create_button({
+            name: 'close',
+            label: text.get('@i18n:buttons.close'),
+            click: function() {
+                dialog.close();
+            }
+        });
+
+
+        return dialog;
+    };
+
+    that.handle_random_otp = function(data, method) {
+        var dialog = that.create_result_dialog();
+        var random_passwd = that.get_field('random').get_value()[0];
+
+        if ((method === 'add' || method === 'add_and_edit') && random_passwd) {
+            var field = dialog.get_field('randompassword');
+            dialog.open();
+            field.load(data);
+        }
+        else if (method === 'add_and_add_another' && random_passwd) {
+            var message = text.get('@i18n:objects.host.generated_otp') +
+                ": " + data.result.result.randompassword;
+            that.show_message(message);
+        }
+    };
+
     that.on_error = rpc.create_4304_error_handler(that);
 
+    that.init();
+
     return that;
 };
 
diff --git a/install/ui/test/data/ipa_init.json b/install/ui/test/data/ipa_init.json
index 1b9b69ff909a9668c1e1867008459d25d5e062a9..6ed7e98246c5d46850718fb1fbfafc0e8aebef0f 100644
--- a/install/ui/test/data/ipa_init.json
+++ b/install/ui/test/data/ipa_init.json
@@ -119,6 +119,7 @@
                         "redirection": "Redirection",
                         "remove_empty": "Select entries to be removed.",
                         "remove_title": "Remove ${entity}",
+                        "result": "Result",
                         "show_details": "Show details",
                         "success": "Success",
                         "validation_message": "Input form contains invalid or missing values.",
@@ -382,6 +383,8 @@
                             "enrolled": "Enrolled",
                             "enrollment": "Enrollment",
                             "fqdn": "Fully Qualified Host Name",
+                            "generate_otp": "Generate OTP",
+                            "generated_otp": "Generated OTP",
                             "keytab": "Kerberos Key",
                             "keytab_missing": "Kerberos Key Not Present",
                             "keytab_present": "Kerberos Key Present, Host Provisioned",
diff --git a/ipalib/plugins/internal.py b/ipalib/plugins/internal.py
index 54871f76de99d92f0f23129b4d636cc4fccfbb8b..e309578af755af814933f8fa20ac55227b3fef63 100644
--- a/ipalib/plugins/internal.py
+++ b/ipalib/plugins/internal.py
@@ -262,6 +262,7 @@ class i18n_messages(Command):
             "redirection": _("Redirection"),
             "remove_empty": _("Select entries to be removed."),
             "remove_title": _("Remove ${entity}"),
+            "result": _("Result"),
             "show_details": _("Show details"),
             "success": _("Success"),
             "validation_title": _("Validation error"),
@@ -527,6 +528,8 @@ class i18n_messages(Command):
                 "enrolled": _("Enrolled"),
                 "enrollment": _("Enrollment"),
                 "fqdn": _("Fully Qualified Host Name"),
+                "generate_otp": _("Generate OTP"),
+                "generated_otp": _("Generated OTP"),
                 "keytab": _("Kerberos Key"),
                 "keytab_missing": _("Kerberos Key Not Present"),
                 "keytab_present": _("Kerberos Key Present, Host Provisioned"),
-- 
2.5.5

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Reply via email to