New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Migrate comment annotations to ElggComment objects (Trac #2146) #2146
Comments
brettp wrote on 40342538-04-14 I'd like to see comments as ElggObjects. The benefits greatly outweigh the possible overhead of the light annotation (which is negated by the problems you mention above). Another thing to consider is an enhanced ElggAnnotation class that is closer to--or is--an ElggEntity. I'm not thrilled with the current implementation of annotations because they are exactly the same as metadata with a few helper functions to do some math. |
ewinslow wrote on 40473773-02-19 +1 for comments as I don't think making annotations into entities is necessary. I've thought the same before, but later realized that we're probably getting this feeling because annotations have been used for things that should be entities (like comments and forum posts). The solution, then, is to use Entities to represent those things, and leave the rest as annotations. If anything, we should think about uniting metadata and annotations since they really are exactly the same in the database (all fields are the same). The thing that's different is the API for interacting with them. To me it seems annotations are used more subjectively whereas metadata is more objective. I.e., annotations typically describe about the relationship between the annotater and the entity (E.g., rating a blog, creating a revision), whereas metadata is typically used for dynamically adding properties to an entity (E.g., the publish_date of the blog). For what it's worth, I think this distinction b/w subjective annotations and objective metadata is useful. |
Title changed from |
cash wrote on 41009457-03-23 This is not going to happen for 1.8. I haven't had the time to take my design and write any code. |
Milestone changed to |
ewinslow wrote on 41097216-04-20 Messageboard posts also warrant this change. Everything that has the potential to generate unique content that might be "likable" should be an entity. |
Milestone changed to |
trac user juho.jaakkola wrote on 42438298-08-11 It hasn't been confirmed yet, but I may be starting a project in August that requires replacing the comment annotations with objects. I'll be doing the project with Elgg 1.8, but maybe it could be possible to make a plugin for 1.8 and use the same code in core of Elgg 1.9. (I'm pretty sure 1.9 won't come out early enough so that I could use it instead of 1.8.) Are there any specifications for the ElggComment yet? I would at least want to make my own implementation as close to the upcoming core functionality as possible so that migration to 1.9 would be easier. |
trac user mrclay wrote on 42472941-01-20 Happy to help here. Is there a spec in progress? |
cash wrote on 42473759-05-21 Three possible approaches:
Downside to adding a new type is 1) adding a new table 2) making it easier to justify adding additional tables :) I suppose a third advantage of a table just for comments is that it divides the heavy data between the objects table and the comments table. I thought perhaps not inheriting from ElggObject would be a bad thing but that class has almost no methods. |
trac user juho.jaakkola wrote on 42473908-03-02 So far approach #3 definitely sounds like the best option. But I'll be happy to see the threading ideas for #2 also. (Besides I didn't get the project, so there's no need to implement anything that could be used also in 1.8 as a plugin.) Btw, would there be any advantages if comments were a plugin in 1.9 instead of core functionality? (Although I suppose there would be only rare cases when anyone would want to actually disable commenting completely. Especially if there are other plugins that use subtyped comments.) |
cash wrote on 42473942-08-12 Re: comments as plugin - Evan has talked about that. I think his primary motivation behind that is giving site owners control over the migration from annotations to entities. (Evan, correct me if I'm wrong here.) I'm hesitant to do the upgrade as a plugin because
|
cash wrote on 42474012-12-04 Everyone on this thread should have access to the Google doc on Elgg Comments (it may have been shared a while ago with you). I was wrong about Evan's reasoning re: comments plugin - it's not about the upgrade but about being able to disable commenting. |
trac user juho.jaakkola wrote on 42478647-02-25 Should I have received a link to the Google doc to my email? |
trac user juho.jaakkola wrote on 42599594-07-17 Couple of questions:
|
trac user juho.jaakkola wrote on 42599833-05-08 No wait... probably more logical in elgg_river would be to have user as subject and comment as object and then get the commented entity with $comment->getContainerEntity()? |
ewinslow wrote on 42604261-04-01 inReplyTo should be the relationship for threading purposes. |
trac user juho.jaakkola wrote on 42604463-06-03 I don't think that the relationship I mentioned is meant for threading. My guess is that it would be used for being able to search for comments for example inside a group without having to loop through all the contents. Is this correct? |
trac user juho.jaakkola wrote on 42621332-05-20 Here's the current state of my comments branch compared to master: I would like to get some feedback about the code. |
trac user juho.jaakkola wrote on 42621344-05-12 Oh, and here's a demo site that uses the comments branch: https://demo.mmg.fi/elgg19/ Feel free to register and try it out. |
trac user mrclay wrote on 42621993-05-30 This is looking really good. Go ahead and send a PR. I forgot we can't get line-level comments from the compare view on forked branches, has to be a PR. |
@juho-jaakkola what's the latest on this? |
Waiting on river API/table changes to be pulled in. |
@mrclay You mentioned you could test the upgrade script with community site data. Have you tried this? If there's too much data for the default upgrade feature we should decide what is the best way to handle the migration. |
Haven't tried, but took a LONG time on a low-traffic site with a few dozen users. I think the clear need is to have the installer only convert the latest day or so of comments then provide an admin page that makes successive XHR requests to replace the rest from latest to oldest. |
Data migrations are my favorite... We need some way to sanity check whether things are happening correctly. There's no visibility into the upgrade process at present and that definitely caused problems when we upgraded the community to 1.8. |
Would something like #4975 require too much work to make it into 1.9? |
!_! just like that... woot |
Original ticket http://trac.elgg.org/ticket/2146 on 40330674-12-21 by cash, assigned to trac user juho.jaakkola.
Elgg version: 1.7
Comments and forum comments are annotations. Annotations do not have a container guid. If you want to do anything with just the comments and forum posts, you need to grab all the entities of a group and then grab all the comments on those entities. This is really slow and ugly.
Two solutions to this:
The text was updated successfully, but these errors were encountered: