Refactoring Comments - let's team up!

Topics: Announcements, Core, Writing modules
Developer
Nov 4, 2011 at 11:53 AM
Edited Nov 7, 2011 at 6:40 PM

Comments is in need of a good modernizing. Who'd like to contribute?

The current aims I can think of are:

  1. Usage of drivers and shapes for every reasonable display and editor building (e.g. make the comment display build dynamically, so attached parts become actually visible)
  2. Usage of BodyPart for the comment text, so comments become easily searchable (which would be possible with the current architecture too, as Sebastien pointed out in the issue linked below) and their editor changeable.
  3. Reactor so that the UserName and email will be only saved separately if the user is not logged in (the current schema would remain, just the fields would be empty if a logged in user has written the comment, then the users table would be joined).
  4. Address this issue (fairly important I think).
  5. Address this issue.
  6. The above points would mean that CommentService has to be also altered, so even its test suite would also need a change.

In this discussion there is also a need for more dynamic comments.

Now these changes would greatly improve the Comments module, however they present a pretty big piece of work. I've started to refactor the Comments module in this fork, aiming to implement the features requested in this issue, but I only started to look into the various aspects, so this code won't even compile. I think those who need a more dynamic Comments module could team up and develop the refactored module together (possible as a new Codeplex project).

What do you think (especially Orchard Team members)? Who'd like to participate?

Nov 4, 2011 at 2:03 PM
I think it would be very useful to be able to attach comments without the need for a routeable part.

Steve



On 4 Nov 2011, at 11:54, "Piedone" <notifications@codeplex.com> wrote:

From: Piedone

Comments is in need of a good modernizing. Who'd like to contribute?

The current aims I can think of are:

  1. Usage of drivers and shapes for every reasonable display and editor building (e.g. make the comment display build dynamically, so attached parts become actually visible)
  2. Usage of BodyPart for the comment text, so comments become easily searchable (which would be possible with the current architecture too, as Sebastien pointed out in the issue linked below) and their editor changeable.
  3. Reactor so that the UserName and email will be only saved separately if the user is not logged in (the current schema would remain, just the fields would be empty if a logged in user has written the comment, then the users table would be joined). I can recall that ther
  4. Address this issue (fairly important I think).
  5. Address this issue.
  6. The above points would mean that CommentService has to be also altered, so even its test suite would also need a change.

In this discussion there is also a need for more dynamic comments.

Now these changes would greatly improve the Comments module, however they present a pretty big piece of work. I've started to refactor the Comments module in this fork, aiming to implement the features requested in this issue, but I only started to look into the various aspects, so this code won't even compile. I think those who need a more dynamic Comments module could team up and develop the refactored module together (possible as a new Codeplex project).

What do you think (especially Orchard Team members)? Who'd like to participate?

Developer
Nov 4, 2011 at 4:17 PM

Now it's not possible to attach CommentsPart to a type that hasn't got RoutablePart?

Nov 4, 2011 at 5:11 PM

I'm pretty sure that's wrong; I already have Comments on content types with no RoutePart and they work fine.

Would be good to see the other issues fixed (I raised that first workitem a while back!)

Coordinator
Nov 4, 2011 at 5:23 PM

I don't think using the Body part is a good idea on the front end, it's not made for that. Keep to the current one please. 

I had investigated the bug from Pete, and as far as I can remember it was a false issue. There were some checks in the controller to prevent the issue raised. But it still need validation, because I didn't close it either.

I already have changed some things in 1.x, don't break this one, or merge the changes in order to work with Projectors.

Ideally I would also like to have a Comments Count property to make requests easier and more optimized, even if it's denormalizing the db.

 

Nov 4, 2011 at 5:56 PM
I tried to do this in 1.2 and it complained. I wanted to add comments for users in their membership area just for each user but it complained about it when removing the route part. I think it's used as a unique identifier.

Ended up creating my own implementation.

Steve



On 4 Nov 2011, at 16:22, "Piedone" <notifications@codeplex.com> wrote:

From: Piedone

Now it's not possible to attach CommentsPart to a type that hasn't got RoutablePart?

Developer
Nov 5, 2011 at 9:19 PM
sebastienros wrote:

I don't think using the Body part is a good idea on the front end, it's not made for that. Keep to the current one please.

Are there any special reasons not to use Body (especially if it has a stripped-down, e.g. Markdown or BBCode editor)? The added benefit of making the editor changeable (or even to be able to set an editor type for Comments) would be great.

Coordinator
Nov 5, 2011 at 10:11 PM

Technical reasons: The BodyPart has be done for admin purposes, and for instance doesn't prevent from using scripts. Also you don't want to provide access to the Media Library. It means a lot of refactoring to remove functionalities. 

Functional reasons: Do you want to allow your users to format their comments ? Can you give me an example where comments can be formatted ? I looked at Disqus, FaceBook comments, Wordpress. No formatting. And I never felt limited not being able to format a comment. At least url replacements, ok, but not formatting.

Also we should keep things simple, because adding complexity where it's not necessary will then require more maintenance. And there are so many things to do which are really necessary right now, we (core developers) need to focus on it. Which doesn't prevent anyone from creating a module aside the Core source code. 

Coordinator
Nov 6, 2011 at 2:35 AM
I second that.

Sent from my TI-99/4A

From: sebastienros
Sent: 11/5/2011 3:11 PM
To: Bertrand Le Roy
Subject: Re: Refactoring Comments - let's team up! [orchard:278333]

From: sebastienros

Technical reasons: The BodyPart has be done for admin purposes, and for instance doesn't prevent from using scripts. Also you don't want to provide access to the Media Library. It means a lot of refactoring to remove functionalities.

Functional reasons: Do you want to allow your users to format their comments ? Can you give me an example where comments can be formatted ? I looked at Disqus, FaceBook comments, Wordpress. No formatting. And I never felt limited not being able to format a comment. At least url replacements, ok, but not formatting.

Also we should keep things simple, because adding complexity where it's not necessary will then require more maintenance. And there are so many things to do which are really necessary right now, we (core developers) need to focus on it. Which doesn't prevent anyone from creating a module aside the Core source code.

Developer
Nov 7, 2011 at 4:45 PM
Edited Nov 7, 2011 at 4:46 PM

Thanks Sebastien, that cleared things up. I thought being able to format comments would be a nice touch, although it can well be that I'm so fixating on a specific problem that I see everything through the same glasses.

Developer
Nov 7, 2011 at 4:48 PM
Edited Nov 7, 2011 at 4:52 PM

I've created a project on Codeplex to organize the work. As I have to upload some source before publishing, a final question: the branch 1.x is the right one then to clone the source?

Coordinator
Nov 7, 2011 at 5:17 PM

Yes, you can safely clone the 1.x, as it will also already contains the changes I have made so far.

Developer
Nov 7, 2011 at 7:02 PM
Edited Nov 7, 2011 at 7:03 PM

I've opened a project here (please tell me if the license is not suitable as it is) Those who want to make contributions please go there. If the development is finished, the module would be added to an Orchard fork.

Developer
May 3, 2012 at 1:16 PM
Edited May 3, 2012 at 1:16 PM

Status update about refactoring the comments module:
Piedone and I will work on this after we're done with our exams, likely in the middle of June. I've created a fork for this purpose.

Coordinator
May 3, 2012 at 4:53 PM

Great !