[Freeipa-devel] [PATCH] 002 Fixed adding host without DNS reverse zone

[Petr Vobornik]

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 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-pvoborni-0002-6-Fixed-adding-host-without-DNS-reverse-zone.patch
Type: text/x-patch
Size: 14375 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20110805/ad8f1c81/attachment.bin>