On 05/12/2016 02:49 PM, Pavel Vomacka wrote: > > > 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
ACK pushed to master: * 3b37e29ac6e918027b06e574c2c793f6c521100c Add option to show OTP when adding host -- Petr Vobornik -- 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
