[Freeipa-devel] [PATCHES] 138-145 Action panel for user password reset

Mon Jun 4 09:04:02 UTC 2012

On 06/01/2012 02:40 AM, Endi Sukma Dewata wrote:
> ACK. Looks good.

Pushed to master.

>
> Some comments:
>
> 1. I suppose the select_action will always be the first action in any
> header_actions, and the action doesn't actually do anything. You might
> want to consider the '-- select action --' as part of the
> IPA.action_list_widget and add it automatically in init_options(), that
> way it doesn't have to be defined explicitly in all header_actions.

Can be. If I do it, I will do it configurable with default to use the 
'-- select action --'.

>
> 2. Ideally the Enable/Disable/Delete actions should only be enabled if
> the user has rights to do that, but this depends on ticket #2187.

Yes. Also, I think enable and disable don't need #2187.

>
> 3. I noticed that the second argument of observer's notify() is always
> the object that owns the observer. For example:
>
> that.observer = IPA.observer();
> that.observer.notify([arg], that);
>
> Since the context doesn't change would it make sense to store the
> context in the observer itself?
>
> that.observer = IPA.observer({ context: that });
>
> The arguments can also be passed as varargs:
>
> that.observer.notify(arg);
> that.observer.notify(arg1, arg2);

I like this idea. It simplifies things.

>
>
> On 5/24/2012 3:54 AM, Petr Vobornik wrote:
>> This bunch of patches implements new concept: action panel and it's
>> implementation in user page.
>>
>> First two patches refactorizes current action-list/control-buttons code
>> to prepare ground for following patches. Sorry for added review work
>> (could be done this way earlier).
>>
>> Patch descriptions:
>>
>> [PATCH] 138 Refactored action list and control buttons to use shared
>> list of actions
>>
>> This is a first step for implementing action panels which will also use
>> the shared list of actions.
>>
>> This effort changes the way how action list and control buttons are
>> defined. First all actions are defined on facet level - attribute
>> 'actions' in spec file. Implementation of action list widget is not
>> specified on facet level. It is left in facet header. A list of action
>> names used in action list can be now specified in facet spec in
>> 'header_actions' attribute.
>> Control buttons use similar concept. Facet by default is using
>> control_buttons_widget. Details and search facet are defining their own
>> default actions (refresh/add/remove/update/reset). Additional buttons
>> can be defined as array of action names on facet level in
>> control_buttons attribute.
>>
>> state_evaluators and state_listeners were united. They are called
>> state_evaluators but they uses state_listener concept, they are attached
>> to an event. For former state_evaluator the event is post_load. They are
>> defined in spec in state attribute. State object purpose is to aggregate
>> states from all state evaluators. It offers changed event to which can
>> other objects subscribe. It also has summary evaluator which evaluation
>> conditions. Summary evaluator creates summary status with human readable
>> description. It can be used by facet header.
>>
>> https://fedorahosted.org/freeipa/ticket/2248
>>
>>
>> [PATCH] 139 Refactored entities to use changed actions concept
>>
>> It's continuation of previous refactoring effort. This part is changing
>> specs in entities to used changed concept.
>>
>> https://fedorahosted.org/freeipa/ticket/2248
>>
>> [PATCH] 140 Action panel
>>
>> This patch implements action panel. Action panel is a box located in
>> facet details section which contains actions related to that
>> object/section.
>>
>> In spec file can be configured actions and title used in action panel.
>> Default title is 'Actions'. Actions are specified by their name. They
>> have to be defined in action collection in facet.
>>
>> https://fedorahosted.org/freeipa/ticket/2248
>>
>>
>> [PATCH] 141 User password widget modified.
>>
>> Currently the user password is shown as follows in the details page:
>> Password: Reset Password
>>
>> This is inconsistent with the rest of the page because the 'Reset
>> Password' is an action, not the value of the password.
>>
>> Now password is shown as follows:
>> Password: ******* (if set)
>> Password: (if not set)
>>
>> Reset password link was removed as well the dialog for reset password
>> was removed from password widget. The dialog was moved to its own object
>> and can be now showed independently. An action for showing this dialog
>> should be created.
>>
>> https://fedorahosted.org/freeipa/ticket/2248
>>
>>
>> [PATCH] 142 Action panel for user
>>
>> This patch adds action panel to user account section. The panel contain
>> an action for reseting user password.
>>
>> https://fedorahosted.org/freeipa/ticket/2248
>>
>> [PATCH] 143 Added missing i18n in action list and action panel
>>
>> This patch adds strings to internal.py which were not translated in
>> action list/panel patches.
>>
>> https://fedorahosted.org/freeipa/ticket/2248
>>
>>
>> [PATCH] 144 Add shadow to dialog
>>
>> This patch adds shadow to dialog used in Web UI. It looks cooler.
>>
>> https://fedorahosted.org/freeipa/ticket/2248
>>
>> note: I didn't want to create new ticket just for this minor visual
>> enhancement.
>>
>>
>> [PATCH] 145 Enable reset password action according to attribute
>> perrmission
>>
>> This patch creates state_evaluator which creates permission states for
>> defined attribute. The state format is: attributeName_permissionChar.
>>
>> This evaluator is used for user_password attribute and it control
>> enabling/disabling of related action in user account action panel.
>>
>> https://fedorahosted.org/freeipa/ticket/2318
>>
>>
>>
>> _______________________________________________
>> Freeipa-devel mailing list
>> Freeipa-devel at redhat.com
>> https://www.redhat.com/mailman/listinfo/freeipa-devel
>
>

-- 
Petr Vobornik