On Thu, 2011-08-04 at 13:24 -0500, Endi Sukma Dewata wrote:
> On 8/4/2011 4:22 AM, Petr Vobornik wrote:
> > new version attached
>
> Almost there, just a few more minor issues.
>
> >> 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 Ajax context right now is only used to get the URL causing HTTP
> error (ipa.js:301). Things might have changed, I'm not sure how to
> generate HTTP error anymore. The URL can actually be obtained from the
> url variable in the execute() method, but there are other things that
> you can get from Ajax context that might be useful in the future. Try
> setting a breakpoint inside the success_handler() or error_handler() and
> inspect the 'this' variable. I think we should make sure the callback
> functions behave like real Ajax callback function to avoid future
> problems, so 'this' should always point to Ajax context.
>
> There are actually a few places where the Ajax context doesn't get
> passed to the callback function:
> - ipa.js:409,418,428,431,436,620
> - host.js:155
> A bunch of these are existing issues. We can fix them separately.
>
> >> 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.
>
> The init() method that we removed recently was a method that was called
> to initialize the object after the metadata becomes available. In the
> past some objects were created before the metadata was available, but
> now it's no longer the case so the object can be created and initialized
> at the same time. There's nothing wrong with creating an init() method
> to encapsulate the initialization sequence, but it doesn't need to be
> made public like before because the subclasses no longer need to call it
> explicitly. No need to change anything here.
>
> The default values in ipa.js:576-579 are redundant because they will be
> overridden by the spec in init().
Removed.
> I think the assignments in init() can
> be replaced by something like this:
> that.xhr = spec.xhr || {};
> Note that the default value for xhr and error_thrown should be an empty
> object.
Reworked, probably we should add some generic error title to internal.py
as default value for error dialog title.
>
> There are some unit test failures in ipa_tests.js because
> IPA.error_dialog used to point to the dialog instance. You might want to
> change it to get the instance using something else, e.g. element ID.
- Added property 'id' to dialog (which is added to its div)
- Added reference to ../dialog.js in ipa.tests.html
- Reworked ipa.test.js to work with error_dialog id.
>
> There are some other other unit test failures, but they seem to be
> caused by the earlier failure. They actually pass if run separately.
>
--> All test should pass.
--
Petr VobornÃk
>From d765145a21e9cbf1b1b6afc2340470f87c179374 Mon Sep 17 00:00:00 2001
From: Petr Vobornik <pvobo...@redhat.com>
Date: Fri, 5 Aug 2011 17:12:21 +0200
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)
Modified core tests to work with dialog.
---
install/ui/add.js | 17 ++++--
install/ui/dialog.js | 32 +++++++++++-
install/ui/host.js | 42 +++++++++++++++
install/ui/ipa.js | 115 ++++++++++++++++++++++------------------
install/ui/test/ipa_tests.html | 1 +
install/ui/test/ipa_tests.js | 23 +++++---
6 files changed, 163 insertions(+), 67 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..0ec84a786160fa82c0b3bf805135cc3e8c2cdb40 100644
--- a/install/ui/dialog.js
+++ b/install/ui/dialog.js
@@ -32,6 +32,7 @@ IPA.dialog = function(spec) {
that.entity = spec.entity;
that.name = spec.name;
+ that.id = spec.id;
that.title = spec.title;
that.width = spec.width || 400;
that.height = spec.height;
@@ -194,7 +195,7 @@ IPA.dialog = function(spec) {
*/
that.open = function(container) {
- that.container = $('<div/>');
+ that.container = $('<div/>', { id : that.id });
if (container) {
container.append(that.container);
}
@@ -257,6 +258,7 @@ IPA.dialog = function(spec) {
that.dialog_create = that.create;
that.dialog_open = that.open;
+ that.dialog_close = that.close;
var fields = spec.fields || [];
for (var i=0; i<fields.length; i++) {
@@ -656,3 +658,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..d53ee7b126a7d5e103013675620dc973aa9f8a0a 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,57 @@ IPA.dirty_dialog = function(spec) {
return that;
};
+
+IPA.error_dialog = function(spec) {
+ var that = IPA.dialog(spec);
+
+ var init = function() {
+ spec = spec || {};
+
+ that.id = 'error_dialog';
+ 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;
+};
diff --git a/install/ui/test/ipa_tests.html b/install/ui/test/ipa_tests.html
index 8f8ab93ef2294aa0e238cc3f182154013565ef5f..b3bf93b16263734379d45316d88bc1a0e7236cc5 100644
--- a/install/ui/test/ipa_tests.html
+++ b/install/ui/test/ipa_tests.html
@@ -11,6 +11,7 @@
<script type="text/javascript" src="../jquery-ui.js"></script>
<script type="text/javascript" src="../jquery.ordered-map.js"></script>
<script type="text/javascript" src="../ipa.js"></script>
+ <script type="text/javascript" src="../dialog.js"></script>
<script type="text/javascript" src="ipa_tests.js"></script>
</head>
<body>
diff --git a/install/ui/test/ipa_tests.js b/install/ui/test/ipa_tests.js
index 72a32783f456944b6fcf3fce336ba89e2948f1df..079b022b3cb5eb24160b3d57bc8d269f316a476d 100644
--- a/install/ui/test/ipa_tests.js
+++ b/install/ui/test/ipa_tests.js
@@ -162,8 +162,10 @@ test("Testing successful IPA.command().", function() {
"Checking ajax invocation counter"
);
+ var dialog = $('#error_dialog');
+
ok(
- !IPA.error_dialog,
+ dialog.length == 0,
"The dialog box is not created."
);
@@ -237,7 +239,8 @@ test("Testing unsuccessful IPA.command().", function() {
on_error: error_handler
}).execute();
- var dialog = IPA.error_dialog.parent('.ui-dialog');
+ var dialog = $('#error_dialog');
+ var ui_dialog = dialog.parent('.ui-dialog');
equals(
ajax_counter, 1,
@@ -245,7 +248,7 @@ test("Testing unsuccessful IPA.command().", function() {
);
ok(
- dialog.length == 1 && IPA.error_dialog.dialog('isOpen'),
+ ui_dialog.length == 1 && dialog.dialog('isOpen'),
"The dialog box is created and open."
);
@@ -255,7 +258,7 @@ test("Testing unsuccessful IPA.command().", function() {
);
// search the retry button from the beginning
- var retry = $('button', dialog).first();
+ var retry = $('button', ui_dialog).first();
retry.trigger('click');
equals(
@@ -270,8 +273,8 @@ test("Testing unsuccessful IPA.command().", function() {
// search the retry button from the beginning again because the dialog
// has been recreated
- dialog = IPA.error_dialog.parent('.ui-dialog');
- retry = $('button', dialog).first();
+ ui_dialog = $('#error_dialog').parent('.ui-dialog');
+ retry = $('button', ui_dialog).first();
retry.trigger('click');
equals(
@@ -286,8 +289,8 @@ test("Testing unsuccessful IPA.command().", function() {
// search the cancel button from the beginning because the dialog has
// been recreated
- dialog = IPA.error_dialog.parent('.ui-dialog');
- var cancel = $('button', dialog).first().next();
+ ui_dialog = $('#error_dialog').parent('.ui-dialog');
+ var cancel = $('button', ui_dialog).first().next();
cancel.trigger('click');
equals(
@@ -295,8 +298,10 @@ test("Testing unsuccessful IPA.command().", function() {
"Checking ajax invocation counter"
);
+ dialog = $('#error_dialog');
+
ok(
- !IPA.error_dialog,
+ dialog.length == 0,
"After cancel, the dialog box is closed."
);
--
1.7.6
_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
