new version attached On Fri, 2011-07-29 at 12:11 -0500, Endi Sukma Dewata wrote: > On 7/29/2011 11:12 AM, Petr Vobornik wrote: > > There was a small error in add.js:162. Fixed! > > Nice job on the dialog boxes. > > There's a problem though, the Retry doesn't quite work. This is because > 'this' object passed to IPA.error_dialog actually points to Ajax context > instead of the IPA.command, so calling execute() on it will fail. Fixed > > When Ajax call returns, it passes a context via 'this' object to the > callback function. The object might contain some useful information > which we would not be able to get any other way. The original code tries > to maintain the context by passing 'this' object along the chain using > call(). Feel free to add comments in the code to clarify this. > > So in dialog_open() you should pass 'that' into the 'command' parameter. > You also need pass 'this' using another parameter so you can use it to > call the error handler if you click Cancel. > > Also these changes should be reverted back to maintain the Ajax context: > > - that.on_error.call(this, xhr, text_status, error_thrown); > + that.on_error(xhr, text_status, error_thrown); > > - that.on_success.call(this, data, text_status, xhr); > + that.on_success(data, text_status, xhr);
Reverted back. Just for my information: ajax context is preserved for some future use, or it is already used somewhere? > The IPA.add_dialog can store the command object as an instance variable > so the IPA.host_adder_dialog can refer to it from the error handler. > > Another thing, in the init() you can access the spec object directly, so > don't really have to pass it as a parameter. Yeah, I know. The purpose for this was to be able to call init method again later (which was made public as xxx_init(spec)). But probably it isn't in compliance with removes of public init methods. petr
>From e142dc9764c8370957888081ebc6bafc611917e7 Mon Sep 17 00:00:00 2001 From: Petr Vobornik <pvobo...@redhat.com> Date: Fri, 29 Jul 2011 10:53:01 -0400 Subject: [PATCH] Fixed adding host without DNS reverse zone https://fedorahosted.org/freeipa/ticket/1481 Shows status dialog instead of error dialog (error 4304 is treated like success). Refactored error dialog. Added generic message dialog (IPA.message_dialog) --- install/ui/add.js | 17 +++++--- install/ui/dialog.js | 28 ++++++++++++ install/ui/host.js | 42 ++++++++++++++++++ install/ui/ipa.js | 119 ++++++++++++++++++++++++++++--------------------- 4 files changed, 149 insertions(+), 57 deletions(-) diff --git a/install/ui/add.js b/install/ui/add.js index 988ea8ff13819ccdd61a2033344e146dbaf09255..b4f1228f04d51893598860261b3bda09d07f8fab 100644 --- a/install/ui/add.js +++ b/install/ui/add.js @@ -31,6 +31,9 @@ IPA.add_dialog = function (spec) { that.method = spec.method || 'add'; that.pre_execute_hook = spec.pre_execute_hook; + that.on_error = spec.on_error ; + that.retry = typeof spec.retry !== 'undefined' ? spec.retry : true; + that.command = null; function show_edit_page(entity,result){ var pkey_name = entity.metadata.primary_key; @@ -51,9 +54,11 @@ IPA.add_dialog = function (spec) { var command = IPA.command({ entity: that.entity.name, method: that.method, + retry: that.retry, on_success: on_success, on_error: on_error }); + that.command = command; pkey_prefix = that.entity.get_primary_key_prefix(); @@ -127,8 +132,8 @@ IPA.add_dialog = function (spec) { var table = facet.table; table.refresh(); that.close(); - } - ); + }, + that.on_error); }); that.add_button(IPA.messages.buttons.add_and_add_another, function() { @@ -141,8 +146,8 @@ IPA.add_dialog = function (spec) { var table = facet.table; table.refresh(); that.reset(); - } - ); + }, + that.on_error); }); that.add_button(IPA.messages.buttons.add_and_edit, function() { @@ -154,8 +159,8 @@ IPA.add_dialog = function (spec) { that.close(); var result = data.result.result; that.show_edit_page(that.entity,result); - } - ); + }, + that.on_error); }); that.add_button(IPA.messages.buttons.cancel, function() { diff --git a/install/ui/dialog.js b/install/ui/dialog.js index 0a1d5428a5b1763cbf4871a9cd59e7415f309c79..dc23db8d04a311d386f44c65c58aacee70c9a13c 100644 --- a/install/ui/dialog.js +++ b/install/ui/dialog.js @@ -656,3 +656,31 @@ IPA.deleter_dialog = function (spec) { return that; }; + +IPA.message_dialog = function(spec) { + + var that = IPA.dialog(spec); + + var init = function() { + spec = spec || {}; + that.message = spec.message || ''; + that.on_ok = spec.on_ok; + }; + + that.create = function() { + $('<p/>', { + 'text': that.message + }).appendTo(that.container); + }; + + that.add_button(IPA.messages.buttons.ok, function() { + that.close(); + if(that.on_ok) { + that.on_ok(); + } + }); + + init(); + + return that; +}; diff --git a/install/ui/host.js b/install/ui/host.js index b8e849211e2a43d9f1a07dc5cb16730f72f034da..743196b08fdcfd9d7c91d92a5f9eba6048b498b2 100644 --- a/install/ui/host.js +++ b/install/ui/host.js @@ -102,6 +102,7 @@ IPA.entity_factories.host = function () { }). standard_association_facets(). adder_dialog({ + factory: IPA.host_adder_dialog, width: 400, height: 250, fields:[ @@ -128,6 +129,47 @@ IPA.entity_factories.host = function () { build(); }; +IPA.host_adder_dialog = function(spec) +{ + spec = spec || {}; + spec.retry = typeof spec.retry !== 'undefined' ? spec.retry : false; + + var that = IPA.add_dialog(spec); + + that.on_error = function(xhr, text_status, error_thrown) + { + var command = that.command; + var data = error_thrown.data; + var dialog = null; + + if(data && data.error && data.error.code === 4304) { + dialog = IPA.message_dialog({ + message: data.error.message, + title: spec.title, + on_ok: function() { + data.result = { + result: { + fqdn: that.get_field('fqdn').save() + } + }; + command.on_success(data, text_status, xhr); + } + }); + } else { + dialog = IPA.error_dialog({ + xhr: xhr, + text_status: text_status, + error_thrown: error_thrown, + command: command + }); + } + + dialog.open(that.container); + }; + + return that; +}; + IPA.host_deleter_dialog = function(spec) { spec = spec || {}; diff --git a/install/ui/ipa.js b/install/ui/ipa.js index 2f1c6969c05c46c646c7796bff092b862157aa8f..4ef7dfda480cf81d2d99666dcbe06616649f6c37 100644 --- a/install/ui/ipa.js +++ b/install/ui/ipa.js @@ -243,57 +243,13 @@ IPA.command = function(spec) { that.execute = function() { function dialog_open(xhr, text_status, error_thrown) { - - IPA.error_dialog = $('<div/>', { - id: 'error_dialog' - }); - - if (error_thrown.url) { - $('<p/>', { - text: 'URL: '+error_thrown.url - }).appendTo(IPA.error_dialog); - } - - $('<p/>', { - html: error_thrown.message - }).appendTo(IPA.error_dialog); - - function close() { - IPA.error_dialog.dialog('destroy'); - IPA.error_dialog.remove(); - IPA.error_dialog = null; - } - - var buttons = {}; - - /** - * When a user initially opens the Web UI without a Kerberos - * ticket, the messages including the button labels have not - * been loaded yet, so the button labels need default values. - */ - var label = IPA.messages.buttons ? IPA.messages.buttons.retry : 'Retry'; - buttons[label] = function() { - close(); - that.execute(); - }; - - label = IPA.messages.buttons ? IPA.messages.buttons.cancel : 'Cancel'; - buttons[label] = function() { - close(); - if (that.on_error) { - that.on_error.call(this, xhr, text_status, error_thrown); - } - }; - - IPA.error_dialog.dialog({ - modal: true, - title: error_thrown.name, - width: 400, - buttons: buttons, - close: function() { - close(); - } + var dialog = IPA.error_dialog({ + xhr: xhr, + text_status: text_status, + error_thrown: error_thrown, + command: that }); + dialog.open(); } function error_handler(xhr, text_status, error_thrown) { @@ -331,6 +287,7 @@ IPA.command = function(spec) { dialog_open.call(this, xhr, text_status, error_thrown); } else if (that.on_error) { + //custom error handling, maintaining AJAX call's context that.on_error.call(this, xhr, text_status, error_thrown); } } @@ -349,11 +306,13 @@ IPA.command = function(spec) { // error_handler() calls IPA.hide_activity_icon() error_handler.call(this, xhr, text_status, /* error_thrown */ { name: 'IPA Error '+data.error.code, - message: data.error.message + message: data.error.message, + data: data }); } else if (that.on_success) { IPA.hide_activity_icon(); + //custom success handling, maintaining AJAX call's context that.on_success.call(this, data, text_status, xhr); } } @@ -610,3 +569,61 @@ IPA.dirty_dialog = function(spec) { return that; }; + +IPA.error_dialog = function(spec) { + var that = IPA.dialog(spec); + + that.xhr = ''; + that.text_status = ''; + that.error_thrown = ''; + that.command = null; + + var init = function() { + spec = spec || {}; + + that.xhr = spec.xhr; + that.text_status = spec.text_status; + that.error_thrown = spec.error_thrown; + that.command = spec.command; + that.title = spec.error_thrown.name; + }; + + that.create = function() { + if (that.error_thrown.url) { + $('<p/>', { + text: 'URL: '+that.error_thrown.url + }).appendTo(that.container); + } + + $('<p/>', { + html: that.error_thrown.message + }).appendTo(that.container); + }; + + that.create_buttons = function() { + /** + * When a user initially opens the Web UI without a Kerberos + * ticket, the messages including the button labels have not + * been loaded yet, so the button labels need default values. + */ + var label = IPA.messages.buttons ? IPA.messages.buttons.retry : 'Retry'; + + that.add_button(label, function() { + that.close(); + that.command.execute(); + }); + + label = IPA.messages.buttons ? IPA.messages.buttons.cancel : 'Cancel'; + that.add_button(label, function() { + that.close(); + if (that.command.retry && that.command.on_error) { + that.command.on_error(that.xhr, that.text_status, that.error_thrown); + } + }); + }; + + init(); + that.create_buttons(); + + return that; +}; -- 1.7.6
_______________________________________________ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
