[Freeipa-devel] [PATCHES] 113-114 Facet expiration flag

[Petr Vobornik]

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.
>
>> 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.

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