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

[Petr Vobornik]

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