On 03/27/2012 05:36 PM, Endi Sukma Dewata wrote:
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.
Pushed to master, ipa-2-2. See comment below.

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.

I think options #1 and #2 doesn't solve the problem when user edits something for a very long time (went for a lunch...). #3 seems as a best solution because it covers single valued attrs too, compare to #4. Which is nice, because user can see what other user entered and may find it better :). Ideal would be to check if data changed before each update, but I think it isn't worth the effort.

I closed the original ticket and moved this to separate ticket. I used your comment as a description. https://fedorahosted.org/freeipa/ticket/2591

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.

Petr Vobornik

Freeipa-devel mailing list

Reply via email to