[Freeipa-devel] [PATCH] HBAC Details Page

[Endi Sukma Dewata]

Hi,

Please take a look at the new patch (also attached):

https://fedorahosted.org/reviewboard/r/99/

On 11/3/2010 1:59 PM, Adam Young wrote:
> Very cool, but suggest we change the term. Would layout perhaps be better?

Renamed that to layout.

>>>> add.js line 34: Do we really need accesor like this? There is nothing
>>>> wrong with doing modifying the member directly. I see the code at line
>>>> 62 that delegates it down the tree...I think there is a more
>>>> javascript-y way to do this. Look up Javascript accessors.

Now it's using setter/getter for entity_name.

>>>> If you are going to change a function header like on associate line
>>>> 133, go ahead and remove the camel_casing as well. (manyObjPKey) as
>>>> you seem to be doing variable cleanup elsewhere.

>>>> Line 297, executor takes 7 params, that are all member variables of
>>>> "that". Since that.execute is invoked as a method, you can remove
>>>> these parameters and instead, internal to executor, refer to them via
>>>> this.<param>

> Yeah. PLus, with the Bulk plugin, we'll want to change the name of the
> bulk associator to something more correct, like single_call versus
> bulk_call, and change the serial associator to use the bulk plugin.

I cleaned up the associators. I added a base class, I also combined the 
adder & deleter (both for serial & bulk) because once the parameters are 
cleaned up they are actually exactly the same code. We can rename these 
classes again later if necessary.

>>>> Typo line 344: that.member_attrribute

Fixed.

>>> Also: remove the buttons for features that we are not going to implement
>>> this time around
>>> from the top of the page: Troubleshoot, Cull Disabled Rules, And the
>>> TEst Rule link under quick links
>>> You can leave Login SVC and Login Svc Groups , those are coming next,
>>> correct?

They are commented out for now, will be added back as we implement them.

>>> Add rule has a rule type field, but no guidance what to fill it in with.
>>> I suspect this should be a select. Without knowing what to put in here,
>>> you can't add a rule. At a minimum, lets put in text 'allow or deny'

Fixed. Will open a new ticket for the drop down list.

Thanks!

-- 
Endi S. Dewata
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-edewata-0025-3-HBAC-Details-Page.patch
Type: text/x-patch
Size: 144967 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20101104/78165a66/attachment.bin>