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

[Pavel Vomacka]

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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-pvomacka-0011-2-Add-option-to-show-OTP-when-adding-host.patch
Type: text/x-patch
Size: 7474 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20160512/305f690b/attachment.bin>