[Freeipa-devel] [PATCH] 137 Entitlement registration.

[Adam Young]

On 04/13/2011 11:03 AM, Endi Sukma Dewata wrote:
> On 4/13/2011 9:12 AM, Adam Young wrote:
>> Please make sure all String literals come from the interal.py messages
>> plugin. New button labels are defined in entitle.js for Consume and
>> Register.
>
> I'm thinking to do that once we have all the entitlement 
> functionalities implemented. There are also hard-coded button labels 
> in ipa.css which I'm not sure how to remove. Any suggestion?

We'll catch the ipa.css changes in a separate patch.  I'll have to take 
a look.

>
>> The consume dialog should be limited to accepting only integer values:
>> pass the validation pattern in to the text widget. that.entity =
>> function(spec) {
>
> There's no pattern defined in the metadata for the quantity. There is 
> an 'int' type though. I think we can add a general purpose type 
> checking in addition to pattern validation. This can be done in a 
> separate patch.

We can add a validation rule [1..9][0..9]* to the entitlement plugin, 
since it should be a positive integer.


>
>> We are starting to see a proliferation of code like this:
>>
>> if (spec instanceof Object){
>> var factory = spec.factory || IPA.entity;
>> entity = factory(spec);
>> } else {
>> var name = spec;
>> entity = IPA.entity({name: name});
>> }
>> return that;
>> };
>>
>>
>> Can we pull it into a helper function?
>
> I checked the code, there are only 2 instances (in entity.js and 
> search.js) where the structures appear to be identical. Other 
> instances contain slight variation which is difficult to extract. I'd 
> suggest we should try to standardize the invocations first then 
> refactor them into a helper function.
Sounds good.

>
>> In dialog.js the code that.entity = IPA.get_entity(that.entity_name);
>> should happen before init, in the initial construction of the object.
>> The goal is to remove init functions; please don't put any more code
>> into them from here on out. Same thing with Facet in entity.js
>
> See IPA.start_entities(). I don't think that's possible because the 
> entity is only added into IPA.entities_by_name in add_entity() while 
> the constructors are executed before that in the factory() invocation.
>
>     factory = that.entity_factories[name];
>     var entity = factory();
>     add_entity(entity);
>     entity.init();
>
> The entity.init() is executed last, that's why IPA.get_entity() for 
> now can only be invoked during the init() phase.
>
> We could pass the entity object instead of entity name in the spec, 
> but that will be a major change.
>

OK,we can punt on this.  We'll work on removing init in a dedicated patch.