[Framework-Team] Review for PLIP 179 - commenting

Wichert Akkerman wichert at wiggy.net
Wed Sep 13 19:09:07 UTC 2006


Introduction
============

This PLIP covers two different issues:

- implement support for replaceable comment implementations using adapters
  and interfaces

- add a new comment implementation which adds the moderate comments and
  filter their html

Both are implemented in the easycommenting product.

easycommenting uses a lot of zope3 technology: interfaces, adapters, browser
views, formlib. This makes it an excellent demonstration of how to use the new
technology within Plone..


Replaceable comments
====================

Replaceable comments are implemented via a browser view for all comment-related
functionality. The commenting system is located through adaption from the a
content type to this interface.

easycommenting replaces plones viewThreadsAtBottom to hook into the standard
plone comments. By using a commentView browser view to access the comments it
makes it possible to support multiple comment implementations.

There are a few issues with the infrastructure:

- since most of easycommenting is about putting a framework in place to
  support pluggable commenting this should be done within the plone 
  namespace, for example in a plone.commenting product.

- comments are not tied to an account: they store the full name, email
  and homepage on all comments. There are several downsides to this
  approach:
  - user informationis duplicated in all comments
  - logged in users need to re-enter their information when adding a comment
  - searching for comments by site member is not possible
  IComment should probably be modified so
  it can deal with something that refers to actual users.

- the interface does not allow for nested comments. Reusing the Ploneboard
  interfaces would add the ability to support nested comments, as well as fix
  the former issue.

- there is a lot of HTML-specific information in ICommentingOptions; it
  may be better to implement these using configurable filters. Ploneboard
  already has a few implementations for these which can be reused.

- the ICommentingOptions interface produced a number of user message;
  should not these be in the user interface itself? A status message or
  text in the template are possible options.

- the viewThreadsAtBottom template assumes that the user interface for
  all implementation is the same. This is not necessarily true and
  warrants a more flexible approach. Perhaps viewlets can be used here?


Commenting implementation
=========================

- there is a basic test suite; it should be expanded to at least cover
  all methods exposed through the defined interfaces

- comments are not included in the SearchableText index, so searching for
  comments is not possible using the default plone search option. If that
  is desirable or not will probably depend on the specific use-case, so this
  is not necessarily a problem. 

- there is a bunch of (ArchgenXML generated?) template code throughout the
  product that looks like it can safely be removed.

- The code uses formlib directlry. This should be changed to use plone.app.form
  instead: this allows you to write cleaner code (you can use zope.formlib
  directly) and the UI will look much more plonish.


Small issues
============

While reviewing I ran into a few minor issues. These are certainly no
showstoppers for merging this bundle but should be fixed before a final
release:

- the toplevel __init__.py still contains a couple of debug statements 
  (such as allow_module("pdb"))

- the descriptions for the fields in IConfigletFormFields are incorrect

- The implementation in adapters.py has does not support translation
  of these messages.

- the skin layer has a readme.txt and tool.gif which should not be there


Verdict
=======
When merging a product into plone core it should be a real in all possible
ways. Currently easycommenting fails this in a few areas which need to be
fixed:

- core interfaces which can be used to hook into standard functionality
  should be in the plone namespace
- configlets should be listed under plone configuration, not under Add-On
  products

The first one is a a definite showstopper and must be fixed before this
bundle can be merged. Additionally I strongly feel the interfaces
need to be revisited. The interfaces from Ploneboard should be a good
basis here.

-- 
Wichert Akkerman <wichert at wiggy.net>    It is simple to make things.
http://www.wiggy.net/                   It is hard to make things simple.




More information about the Framework-Team mailing list