Discussion tool fails when user explicitly disables discussion

[Tres Seaver]

f a user chooses "Disabled" on "allow discussion on this item" on a story, the object saves with it set to "Enabled".

(Default works correctly)

----
Root cause:
-SelectionWidget (used for allowDiscussion 3-way) must use strings for vocabulary, so, we use '0', '1', and 'None' for disabled, enabled and default.

The string '0' and the string '1' are both Boolean "True", but Products.CMFDefault-2.2.2-py2.7.egg/Products/CMFDefault/DiscussionTool.overrideDiscussionFor
 simply does a 'bool(allowDiscussion)' assuming it's an int.

So, disabled ('0') sets the object allow_discussion to True, incorrectly.

---------
Suggested FIX:
Products.CMFDefault-2.2.2-py2.7.egg/Products/CMFDefault/DiscussionTool.py

overrideDiscussionFor - line 84

Replace:

else:
   content.allow_discussion = bool(allowDiscussion)

With:
 else:
   content.allow_discussion = bool(int(allowDiscussion))

[Tres Seaver (tseaver)]

The attached patch is a failing unit test demostrating the bug.

[Tres Seaver (tseaver)]

Released with 2.2.3:

  http://pypi.python.org/pypi/Products.CMFDefault/2.2.3

[Maurits van Rees (maurits-vanrees)]

There is a similar problem when you export and import content via portal_setup. The export tarball will have a line with:

allowDiscussion: False

(Or True of course.) When you import this tarball you get a traceback:

2013-01-17 18:42:38 ERROR Zope.SiteErrorLog 1358444558.710.826857902685 http://127.0.0.1:8080/Plonelowerprop/portal_setup/manage_importTarball
Traceback (innermost last):
  Module ZPublisher.Publish, line 126, in publish
  Module ZPublisher.mapply, line 77, in mapply
  Module ZPublisher.Publish, line 46, in call_object
  Module Products.GenericSetup.tool, line 565, in manage_importTarball
  Module Products.GenericSetup.tool, line 350, in runAllImportStepsFromProfile
  Module Products.GenericSetup.tool, line 1100, in _runImportStepsFromContext
  Module Products.GenericSetup.tool, line 1015, in _doRunImportStep
   - __traceback_info__: content
  Module Products.CMFCore.exportimport.content, line 34, in importSiteStructure
  Module Products.CMFCore.exportimport.content, line 190, in import_
  Module Products.GenericSetup.content, line 408, in import_
  Module Products.Archetypes.WebDAVSupport, line 121, in PUT
  Module Products.Archetypes.utils, line 123, in mapply
  Module Products.Marshall.marshaller, line 107, in demarshall
  Module Products.Marshall.marshaller, line 90, in delegate
   - __traceback_info__: (<Products.Archetypes.Marshall.RFC822Marshaller instance at 0x10467e170>, 'demarshall', <ATDocument at /Plonelowerprop/page>, {'mimetype': None, 'REQUEST': <Products.GenericSetup.content.FauxDAVRequest instance at 0x1069396c8>, 'RESPONSE': <Products.GenericSetup.content.FauxDAVResponse instance at 0x1069390e0>, 'context': <ATDocument at /Plonelowerprop/page>, 'filename': 'page'})
  Module Products.Archetypes.utils, line 123, in mapply
  Module Products.Archetypes.Marshall, line 236, in demarshall
  Module Products.Archetypes.ExtensibleMetadata, line 287, in allowDiscussion
  Module Products.CMFDefault.DiscussionTool, line 82, in overrideDiscussionFor
ValueError: invalid literal for int() with base 10: 'False'

Maybe it needs to be fixed somewhere else, but it works as expected for other boolean fields as far as I see.
So it seems easiest to add a fix in CMFDefault. I will do that on branch 2.2 and trunk.

[Maurits van Rees (maurits-vanrees)]

Done in r129047 on branch 2.2 and merged to trunk. This was the change, in DiscussionTool.py:

+ if allowDiscussion in ('True', 'true', 'on'):
+ allowDiscussion = True
+ elif allowDiscussion in ('False', 'false', 'off'):
+ allowDiscussion = False
             content.allow_discussion = bool(int(allowDiscussion))

[Simone Orsi (simone-orsi)]

I am still experiencing this on https://pypi.python.org/pypi/Products.CMFDefault/2.2.3#id9 with Plone 4.2.4
I do not see the chage Maurits is point out. Is there an upcoming release? (latest beta releases do not include it neither).

[Simone Orsi (simone-orsi)]

Just tried to use trunk but looks like is not an option ATM since I get:

zope.configuration.xmlconfig.ZopeXMLConfigurationError: File "/home/simahawk/dev/plone/plumi/plumi_newbuildout/parts/instance-1/etc/site.zcml", line 16.2-16.23
    ZopeXMLConfigurationError: File "/home/simahawk/dev/plone/plone4/buildout/eggs/Products.CMFCalendar-2.2.2-py2.7.egg/Products/CMFCalendar/configure.zcml", line 11.2-11.31
    ZopeXMLConfigurationError: File "/home/simahawk/dev/plone/plone4/buildout/eggs/Products.CMFCalendar-2.2.2-py2.7.egg/Products/CMFCalendar/browser/configure.zcml", line 39.2-46.8
    ConfigurationError: ('Invalid value for', 'class', 'ImportError: Module Products.CMFDefault.browser.utils has no global MacroView')

[Tres Seaver (tseaver)]

Simone:

> I am still experiencing this on https://pypi.python.org/pypi/Products.CMFDefault/2.2.3#id9 with Plone 4.2.4.

Is that the export / import bug Maurits pointed out, or the one I reported (and think I fixed in CFMDefault 2.2.3) where the checkbox didn't work?

> Just tried to use trunk but looks like is not an option ATM since I get:

You will need to update the other products (CMFCalendar, CMFCore, CMFTopic, CMFUid, DCWorkflow, GenericSetup) to work with the trunk.