[Freeipa-devel] [PATCH] 030 Extending facet's mechanism of gathering changes

[Petr Vobornik]

On 11/03/2011 11:51 PM, Endi Sukma Dewata wrote:
> On 11/3/2011 7:35 AM, Petr Vobornik wrote:
>> https://fedorahosted.org/freeipa/ticket/2041
>>

> Other issues:
>
> 1. In details.js:650 we don't use param_info anymore, it should be
> metadata.
Fixed
>
> 2. The add_field_option(command, field_name, param_info, values, join)
> probably can be simplified into add_field_option(field, values). The
> IPA.command_builder can store the command internally:
>
> var command_builder = IPA.command_builder({
> command: command
> });
>
> ...
>
> command_builder.add_field_option(field_info.field, values);
>
> The add_field_option() can get the name, metadata, and join from the
> field object.

Transformed to following usage:
IPA.command_builder.add_field_option(command, field, values);

I'm considering command builder more as an utility class, than proper 
builder. If it would gather more functionality it would be better to 
changed it that way.

> 3. The add_record_to_command() takes a list of sections, but it might
> not be necessary because (so far) it's only used to access the details
> facet's own list of sections. Do you expect this to be used differently?

Removed (4). But you were right. The parameter would be nice if it was 
in some utility class, not details facet where different usage would be 
unlikely.

> 4. The create_fields_update_command() is essentially the same as
> create_standard_update_command(). When the command_mode is 'save' is it
> possible to generate an update_info from records so we can just call
> create_fields_update_command()?

Created save_as_update_info(only_dirty, require_value) method which 
should do the trick.

It internally use save(record) method do get all data and the parameters 
are used to get only the changes. It allowed to delete 
add_record_to_command and create_fields_update_command methods.
>
> This patch I think can be ACKed after fixing #1, the rest can be dealt
> with later.
>
> Some of the new codes are not used anywhere yet so it's a bit difficult
> to review and things might change again later. I'm curious to see how
> it's going to be used in HBAC/sudo rule and DNS zone that involves
> multiple commands: how the commands are going to be defined, how to
> associate certain fields with certain commands, how to assign
> priorities, etc. Do you have any preview patch for ticket #1515?

Attached preview patch for #1515. Also attaching diff patch of reviewed 
patch.
-- 
Petr Vobornik
-------------- next part --------------
A non-text attachment was scrubbed...
Name: wip-freeipa-pvoborni-0013-Review-changes.patch
Type: text/x-patch
Size: 6768 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20111104/15863f85/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: wip-freeipa-pvoborni-0014-Code-cleanup-of-HBAC-Sudo-rules.patch
Type: text/x-patch
Size: 67669 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20111104/15863f85/attachment-0001.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-pvoborni-0030-1-Extending-facet-s-mechanism-of-gathering-changes.patch
Type: text/x-patch
Size: 16346 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20111104/15863f85/attachment-0002.bin>