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 -- 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