[Framework-Team] Re: Review for PLIP 179 - commenting

Kai Diefenbach kai.diefenbach at iqpp.de
Sat Sep 16 11:07:30 UTC 2006


Hi Wichert,

(should I have this sent to framework-team at lists.plone.org, also?)

Am 13.09.2006 um 21:09 schrieb Wichert Akkerman:
> 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.

Reasonable. No problem from my side.

> - 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

Yes, that's true. A possible solution could be just to save the  
member id for a authenticated member, but then we should think about  
the case a member will be deleted.

>   - logged in users need to re-enter their information when adding  
> a comment

No, logged in users don't even see a name or e-mail field, but I see  
your point mentioned above.

>   - searching for comments by site member is not possible

Yes. See above.

>   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.

Yes. nested comments are on the top of my todo list.

Generally, we should decide, whether a comment should be a AT or just  
a simpler datatype like a dict or somethink like that. I think former  
would bring a lot of advantages:
	
	- easy realisation of nested comments
	- easy using of workflows, hence easy realisation of moderation
	- easier to search (?)
	- using of default CRUD mechanism

My first approach was realised with an AT, but then optilude remarks,  
that this could be a little bit overloaded (http://tinyurl.com/ 
fckj5), so I tried an approach with dicts. But as I write these lines  
I would prefer to go back to AT. What do you think?

> - 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.

Okay, I will check this, too. But this could lead to a problem if one  
want to use different options on different objects or products which  
uses all plone.commenting. Or do I miss a point? With  
ICommentingOptions approach one could write own adapters, which would  
then use side wide options or own ones for every product or even object.

> - 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 initial idea was to provide a generic, easy to use component  
which could be easily reused in different contextes. So my thought  
was, that a user of the commenting framework is able to change  
message for a special "comment" type. For example:

	- Comment
	- Blog Entry
	- Guestbook Entry
	- Note
	- ...

And I don't know how this could be solved for example with the i18n- 
framework, if the component is used more the one times in different  
contextes in one Plone site. But there might be other, better ways,  
of course.

> - 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?

Yes, I see, as I have heard a talk about viewlets from Phillipp, just  
today :-).

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

In planning.

> - 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.

Yes it can. I'll do that, as I've decided to remove ArchGenXML code  
from all of my products, recently.

> - 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.

Have to confess, that I never heard of it before, but I will dive in.

> 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

Oha, that's true :-(

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

The support for translations is in the AddCommentForm at the moment.  
Do you consider this as wrong/bad style?

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

All this issues are easily to fix.

> 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.

Both could be easily fixed from my point of view.

Hope that helps. Generally spoken, I'm keen to improve easycommenting  
or better plone.commenting to land it in Plone. I would propose that  
I start immediately with the following steps as these should be done  
without general decisions:

	- Change namespace to plone.commenting
	- Remove ArchGenXML code
	- Fix bugs as described in small issues above
	- Change to plone.app.form
	- Adding more and better tests

Then we need some decisions about the content type for comments and  
which approach we want to use for options and/or HTML handling.
What do you mean?

Greetings
Kai




More information about the Framework-Team mailing list