hi alec, hi patrick, On 24.02.2010, at 19:31, Alec Mitchell wrote: >> 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? :)
first of all, i didn't get round to looking at it on friday. sorry. > 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. aha, so it doesn't actually do a catalog search here, right?! i was wondering why `c.indexing`'s auto-flushing didn't kick in here (it processes the indexing queue on catalog searches as some sort of safety net)... > 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). right, but i think it's a bad idea to rely on `reindexObject` updating the modification time. i mean, `reindexObject` shouldn't do that in the first place, because the object is actually _not_ modified when it's being reindexed. the time should be updated in the code that does the modification instead, i.e. in `update` or `processForm`, don't you think? > 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. aah, that already answers it +1!! :) > 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). right, and i think letting CMFEditions (or other packages) check `ModificationDate` is fine, it's just that the catalog shouldn't be responsible for updating it. like we already established :) > It's not surprising that the tests don't pick up this > failure since the tests don't test anything quite so pathological. :) patrick, would you mind creating a ticket for this? cheers, andi -- zeidler it consulting - http://zitc.de/ - [email protected] friedelstraße 31 - 12047 berlin - telefon +49 30 25563779 pgp key at http://zitc.de/pgp - http://wwwkeys.de.pgp.net/ plone 4.0 alpha released! -- http://plone.org/products/plone/ _______________________________________________ Product-Developers mailing list [email protected] http://lists.plone.org/mailman/listinfo/product-developers
