[Freeipa-devel] [PATCH] 0011 webui: Offer OTP generation for host enrollment in the UI

[Petr Vobornik]

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;
});


-- 
Petr Vobornik