Description

I've just enabled versioning for files (in setup/types). I now have a history tab for pre-existing files, but when I try to access them I get an error message (traceback attached below). If I add a new file, it's history tab works as expected. The problem seems to be with files which were added to plone before enabling versioning.

Traceback (innermost last):

Module ZPublisher.Publish, line 119, in publish
Module ZPublisher.mapply, line 88, in mapply
Module ZPublisher.Publish, line 42, in call_object
Module Shared.DC.Scripts.Bindings, line 313, in call
Module Shared.DC.Scripts.Bindings, line 350, in _bindAndExec
Module Products.CMFCore.FSPageTemplate, line 216, in _exec
Module Products.CacheSetup.patch_cmf, line 51, in FSPT_pt_render
Module Products.CacheSetup.patch_cmf, line 126, in PT_pt_render

Warning: Macro expansion failed

Warning: exceptions.KeyError: 'view_macro'

Module zope.tal.talinterpreter, line 271, in call
Module zope.tal.talinterpreter, line 346, in interpret
Module zope.tal.talinterpreter, line 891, in do_useMacro
Module zope.tal.talinterpreter, line 346, in interpret
Module zope.tal.talinterpreter, line 536, in do_optTag_tal
Module zope.tal.talinterpreter, line 521, in do_optTag
Module zope.tal.talinterpreter, line 516, in no_tag
Module zope.tal.talinterpreter, line 346, in interpret
Module zope.tal.talinterpreter, line 957, in do_defineSlot
Module zope.tal.talinterpreter, line 346, in interpret
Module zope.tal.talinterpreter, line 536, in do_optTag_tal
Module zope.tal.talinterpreter, line 521, in do_optTag
Module zope.tal.talinterpreter, line 516, in no_tag
Module zope.tal.talinterpreter, line 346, in interpret
Module zope.tal.talinterpreter, line 861, in do_defineMacro
Module zope.tal.talinterpreter, line 346, in interpret
Module zope.tal.talinterpreter, line 957, in do_defineSlot
Module zope.tal.talinterpreter, line 346, in interpret
Module zope.tal.talinterpreter, line 536, in do_optTag_tal
Module zope.tal.talinterpreter, line 521, in do_optTag
Module zope.tal.talinterpreter, line 516, in no_tag
Module zope.tal.talinterpreter, line 346, in interpret
Module zope.tal.talinterpreter, line 949, in do_defineSlot
Module zope.tal.talinterpreter, line 346, in interpret
Module zope.tal.talinterpreter, line 586, in do_setLocal_tal
Module zope.tales.tales, line 696, in evaluate

Change History

This is caused by the fact that in ZVCStorage.py getHistoryMetadata returns and empty list in the case of a non-existent history, but the template (versions_history.pt) assumes that (a) a history item will always be returned, and (b) that the history is a LazyHistory, which implements a 'getLength' function, instead of just using the builtin len. This could be solved by either:

a) testing for 'truthiness' of history before trying to use methods/attributes on it in versions_history.pt

b) returning an empty shadowhistory object. this looks tricky because of the assumptions baked into the code

c) changing policy of len implementation to set countPurged=False, and then just use len(history) in the template versions_history.pt

Actually, after a second look at the code, it seems to me that best/most elegant solution, is simply to do away with the various tests in ZVCStorage.ZVCStorageTool.getHistoryMetadata, and instead simply do:

return self._getShadowHistory(history_id, autoAdd=True)

Also, while I'm here, the code in CopyModifyMergeRepository.CopyModifyMergeRepositoryTool.getHistoryMetadata looks a little hokey - why the special test to decide whether to wrap hist in ImplicitAcquisitionWrapper or not? Probably that should be junked with a simple: return ImplicitAcquisitionWrapper(hist, self) ...regardless of what 'hist' is (None, an empty list, or an empty ShadowHistory...)

Woooweee, this just gets better and better; further to the suggestion above, plone.app.layout.viewlets.content, in getRevisions has a special shortcut where they comment that 'history might be an empty list' and then shortcut to simply return it. This breaks when history is now (correctly) and empty ShadowHistory object, so this special test and shortcut should be removed.

I think solution a) is by far the simplest and involves a single simple change to the version_history_form. The ShadowHistory is a persistent object and I'm not comfortable constructing and returning a dummy empty version of it when the return of _getShadowHistory is None. Just passing the None slightly simplifies the getHistoryMetadata method, but would require changes elsewhere.

This has been fixed in collective r111460 and merged into the 1.2 branch in r111461.