[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