[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