On 3/22/2012 12:50 PM, Petr Vobornik wrote:
The effort was split to two patches. See patch descriptions.

ACK on both. Nice job. See comments below.

Notes:
Case #7 (automatic: an open facet should automatically refresh itself
when it expires) I don't want to implement because I think it can cause
havoc ie: refresh when user is editing.

For details page, automatically refreshing the page still makes sense to avoid overriding newer data with older information. For example you open a user details page to add an email address, but at the same time somebody else added another email address to same user. When you finally save your changes the other email address will be gone.

I think there are several options:
1. Don't refresh if the page is dirty (i.e. user is editing the page).
2. Refresh unedited fields only.
3. Load the data even though it's being edited, compare with the cached data. If something's changed alert the user. The user can either revert the changes or continue editing.
4. Use addattr/delattr to modify multi-valued attributes.

The chance of this happening is probably small, and the solution won't completely eliminate the risk either, so this is probably lower priority.

For search page, there are 2 things that the page keeps in cache: the primary keys and the visible entry details. Currently when you change page it will refresh both, so periodically refreshing the page may not be that important. However ideally we shouldn't need to refresh the primary keys in all page changes because it could be long and most likely unchanged. In this case it makes sense to refresh the primary keys periodically. Probably separate ticket.

Also I was thinking about splitting needs_update to two methods:
needs_update and state_changed. State changed would compare current
state with old state. It would be called from needs_update. This way we
don't have to override needs_update and just define various
state_changed for different facets.

Feel free to refactor the methods if you think it's necessary. But before that try to rearrange the current implementations of needs_update() and minimize the amount of code that needs to be moved into state_changed().

Also I'm thinking about adding
paging state comparison to search facet (the page should recognize the
change itself ie when other facet redirects to certain page (bad
example?)). What do you think?

Yes, this would be needed if we decided to keep the primary keys for a longer time like described above. Changing the page should refresh the visible entry details only.

--
Endi S. Dewata

_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to