forwarding on behalf of alec as the cc to the list got lost...
-------- Original Message -------- Subject: Re: Review of a branch that should make CMFEditions and collective.indexing work together peacefully Date: Wed, 24 Feb 2010 10:31:02 -0800 From: Alec Mitchell <[email protected]> To: Andreas Zeidler <[email protected]> On Wed, Feb 24, 2010 at 5:09 AM, Andreas Zeidler <[email protected]> wrote: > On 22.02.10 16:54, Patrick Gerken wrote: >> Hi, > > hi patrick, > >> I have created a test case and a change in behavior that fixes the issue. >> Instead of comparing the modified date of new and old object, I check for the >> _p_changed attribute. That is a rather drastic change but adding an >> assertion in >> the change to compare old modification check vs new modification check >> showed that both yield the same results in all test cases when used without >> collective.indexing. > > great. could you please elaborate a bit more on what the problem was > here, though? > >> I would be rather grateful if somebody with a bit of >> collective.indexing knowledge could have >> a look at it, maybe there is a better way than the one I implemented. >> My changes are in the do3cc_collective.indexing branch. > > i can have a look on friday, but i'm not sure i'm on top of the > CMFEditions part. i'm cc'ing alec in the hope he might be able to join > the tuneup? :) My understanding is that the combination of collective.indexing's delayed indexing and the fact that AT sets the modification date on reindex caused CMFEditions to think that newly modified content was not actually modified and prevented saving new versions. Essentially, the CMFEditions script that saves new versions on edit uses a method portal_modifier.isUpToDate(obj) to check if the object has actually changed since the last save, if not it doesn't bother to save a new version (one can always manually save a version without this check though). Because CMFEditions is intended for use with CMF content it uses the standard ModificationDate() method to determine if the object was modified since the last saved version. That interaction between collective.indexing and AT means that the date is not changed until the transaction is over and so the indexing does not happen (or happens only every other save). I would not call this a bug in CMFEditions though, since the change to the behavior of ModificationDate really is breaking expectations in a problematic way. I wouldn't be surprised if other products and applications rely on checks to ModificationDate within a transaction, so I suggest we bind that change to some more reliable event than the reindexing action itself. Using _p_changed (or more appropriately bobobase_modification_time) may work in some cases, but it is not equivalent and will certainly fail in a number of fairly common cases (e.g. when all the changed fields are stored in an annotation storage). It's not surprising that the tests don't pick up this failure since the tests don't test anything quite so pathological. Alec _______________________________________________ Product-Developers mailing list [email protected] http://lists.plone.org/mailman/listinfo/product-developers
