How can I get the next or previous comment in a page ?

alblon · May 2017

Hi everybody,
this is the first time I write here. I've been lurking around for two months now, learning how to develop my own plugins and applications with Vanilla Forums.
I'm developing a gallery application for my forum and thanks to all your past questions and answers I've solved nearly all my problems and the application is working quite well.
Now I've a problem that it's seems nobody have already had in the past.

I have to display images from my gallery in between comments. As you know, when there are more than 30 comments (the limit you set in the dashboard) in the discussion, the comments are displayed in pages. If for example I'm displaying the second page, the comments from number 31 to number 60 are displayed. I need to read (but not to display) from the DB the comment number 30 and number 61, i.e. the comment just before the first comment of the page and the comment just after the last comment of the page. I need them to work out which images to show

The best I've been able to do is the following. To read the first comment after the last one of the page I do this:

public function DiscussionController_AfterComments_Handler($Sender, $args) {
   $Offset = $Sender->CommentModel->getOffset($Sender->CurrentComment->CommentID);
   $Limit = 1;
   $Result = $Sender->CommentModel->getByDiscussion($Sender->DiscussionID, $Limit, $Offset + 1);
   // Do whatever I need with the result
}

I can write a similar code to read the comment before the first one of the page.
Is this the best way to read those comments ?
Any better idea ?

Thank you !
--Alberto

R_J · May 2017

A warm welcome and congrats for your success so far!

I'm impressed from the solution you were already giving since that looks quite good. There are five things that come to my mind when looking at that problem:

1. Database caching

When you open a page a query is made and when you do exactly the same query, chances are high that the same query has been cached and has no impact. The way you do this creates a new query which is completely "useless" for any other reason.
If you would not limit the result to 1 row, but instead query page one and page three if you are at page two by using the comment models getByDiscussion method, you would initiate the query that the user most probably has made right before the current page view and which he will most probably do right after this page view.
If you attach a ->resultArray() after the getByDiscussion call, you would be able to retrieve the first and the last comment very easily.

2. Simple SQL

Have you taken a look at the getByDiscussion method? The query there is quite complex and includes a lot of extra information which isn't needful for you. I would consider that method a "heavy" method and not use it if there weren't any benefits in there.
A simple SELECT * FROM Comment WHERE CommentID < (lowest comment ID displayed) and DiscussionID = (the current discussions DiscussionID))->lastRow(), would do the trick. Maybe ordering by desc CommentID and limitting the results to 1 would be even better.
Compared to what is happening in in getByDiscussion, that is really lighweight.

3. Change the initial query to include your results

I'm not sure that this would really work, but it could be worth a thought...
Use the "BeforeGet" event in the CommentModels getByDiscussion method. The limit as well as the offset is passed to that event and can be influenced by you. So you could check if the offset is 0 (you are on the first page). If yes, increase the limit by one to include the first comment from page two. If the offset is above zero, increase limit by two and decrease offset by one to also include the last comment of the previous page.
Use the AfterGet event to strip off both of your comments from the resulting dataset and use them as you need to.

4. Whatever you do: would caching be an option?

This is not a solution, just a hint. When 100 people look at a discussion and that results in some more queries, there is no problem. But wouldn't it be great if your plugin would be used in a forum for 100k members or even more? It should scale well, then.
Scaling is best done by using some sort of precalculation and caching.
Precalculation: each comment has an "Attributes" column where you could store "meta" information. Maybe you could use that? Would adding a "PreviousCommentID"/"NextCommentID" be helpful? If yes, don't forget to handle splitted discussions and deleted comments! And whenever you need that information, see if the info is in the first and last comment of the current page. If not, determine if it has to be fetched and if yes, get it and write it to the Attribute column so that it must not be fetched any more.

Caching: if adding meta information to comments isn't helpful in your case, think of caching the result of your query.

$LastCommentID = Gdn::cache()->get('LastCommentIDDiscussion42Page3');
if ($LastCommentID === Gdn_Cache::CACHEOP_FAILURE) {
    $LastCommentID = (sql result)
    Gdn::cache()->store(
        LastCommentIDDiscussion42Page3,
        $LastCommentID,
        [Gdn_Cache::FEATURE_EXPIRY => 3600] // 1 hour
    );
}

That way the impact on your database should be quite low, to. If you often delete comments or split discussions, you should consider using a smaller amount for the cache, since invalidating it with such a cache key is quite uncomfortable.

5. Why? ;-)

Is that really needed? What is it that you try to achieve? You only said what you try to do and not why you want to do so. Maybe there is a better approach if you could explain the reason why you want to fetch those comments.

And without knowing the reason why, I think that I'm not able to advice what's best. But I'm a fan of precalculation. In this case deleting a comment would require a somewhat complicated treatment, but since the benefit of precalculation is that you only have to make extra queries twice per discussion page and afterwards the required info will always be available, would make this my favorite solution.

R_J · May 2017

A warm welcome and congrats for your success so far!

I'm impressed from the solution you were already giving since that looks quite good. There are five things that come to my mind when looking at that problem:

1. Database caching

When you open a page a query is made and when you do exactly the same query, chances are high that the same query has been cached and has no impact. The way you do this creates a new query which is completely "useless" for any other reason.
If you would not limit the result to 1 row, but instead query page one and page three if you are at page two by using the comment models getByDiscussion method, you would initiate the query that the user most probably has made right before the current page view and which he will most probably do right after this page view.
If you attach a ->resultArray() after the getByDiscussion call, you would be able to retrieve the first and the last comment very easily.

2. Simple SQL

Have you taken a look at the getByDiscussion method? The query there is quite complex and includes a lot of extra information which isn't needful for you. I would consider that method a "heavy" method and not use it if there weren't any benefits in there.
A simple SELECT * FROM Comment WHERE CommentID < (lowest comment ID displayed) and DiscussionID = (the current discussions DiscussionID))->lastRow(), would do the trick. Maybe ordering by desc CommentID and limitting the results to 1 would be even better.
Compared to what is happening in in getByDiscussion, that is really lighweight.

3. Change the initial query to include your results

I'm not sure that this would really work, but it could be worth a thought...
Use the "BeforeGet" event in the CommentModels getByDiscussion method. The limit as well as the offset is passed to that event and can be influenced by you. So you could check if the offset is 0 (you are on the first page). If yes, increase the limit by one to include the first comment from page two. If the offset is above zero, increase limit by two and decrease offset by one to also include the last comment of the previous page.
Use the AfterGet event to strip off both of your comments from the resulting dataset and use them as you need to.

4. Whatever you do: would caching be an option?

This is not a solution, just a hint. When 100 people look at a discussion and that results in some more queries, there is no problem. But wouldn't it be great if your plugin would be used in a forum for 100k members or even more? It should scale well, then.
Scaling is best done by using some sort of precalculation and caching.
Precalculation: each comment has an "Attributes" column where you could store "meta" information. Maybe you could use that? Would adding a "PreviousCommentID"/"NextCommentID" be helpful? If yes, don't forget to handle splitted discussions and deleted comments! And whenever you need that information, see if the info is in the first and last comment of the current page. If not, determine if it has to be fetched and if yes, get it and write it to the Attribute column so that it must not be fetched any more.

Caching: if adding meta information to comments isn't helpful in your case, think of caching the result of your query.

$LastCommentID = Gdn::cache()->get('LastCommentIDDiscussion42Page3');
if ($LastCommentID === Gdn_Cache::CACHEOP_FAILURE) {
    $LastCommentID = (sql result)
    Gdn::cache()->store(
        LastCommentIDDiscussion42Page3,
        $LastCommentID,
        [Gdn_Cache::FEATURE_EXPIRY => 3600] // 1 hour
    );
}

That way the impact on your database should be quite low, to. If you often delete comments or split discussions, you should consider using a smaller amount for the cache, since invalidating it with such a cache key is quite uncomfortable.

5. Why? ;-)

Is that really needed? What is it that you try to achieve? You only said what you try to do and not why you want to do so. Maybe there is a better approach if you could explain the reason why you want to fetch those comments.

And without knowing the reason why, I think that I'm not able to advice what's best. But I'm a fan of precalculation. In this case deleting a comment would require a somewhat complicated treatment, but since the benefit of precalculation is that you only have to make extra queries twice per discussion page and afterwards the required info will always be available, would make this my favorite solution.

alblon · May 2017

Hi R_J !
Thank you very much for your very detailed answer!

First of all, what I'm trying to accomplish ? As I told before, it's a gallery application with which the forum users are able to publish their images organized in albums. It started as a simple gallery of images, but soon I wanted to give users the ability to comment other users albums. So I decided to link a discussion to an album.
Given the nature of the forum (a scale model builder forum), albums can be updated by adding more photos (they often represent a work in progress of a model building). Those new photos need to be showed between comments, in an order given by the date-time in which they are inserted. So a bunch of photos inserted today have to be showed after a comment inserted yesterday, but before a comment inserted tomorrow.
This way albums and the relative comments evolve together.
I hope that I've been clear with my shaky English.

But I want to reply to your points.
I agree with you that using the getByDiscussion method for reading just 2 records is too much. That's the reason why I decided to ask an opinion here.

1. Database caching

Reading up to three pages instead of one, relying on the cache functionality is an interesting idea. But reading that much data is something I don't like much.

2. Simple SQL

This obviously was my first idea. Simple and effective. But I wanted to use the CommentModel if possible, avoiding to access directly to the database. Probably I'll fall back to this solution if other ones will prove being ineffective.

3. Change the initial query to include your results

Believe it or not, this was my second idea, and probably the best one because it reads two more records per page from the DB without additional queries. Moreover it can make use of the caching feature.
What held me from following this route is that it was like tampering too much with that method. I don't know if the "BeforeGet" and "AfterGet" events can really be used this way. Moreover there are other methods that fire those two events: the "BeforeGet" is also fired by the getByUser method and the "AfterGet" is also fired by the getByUser2 method. How can I safely distinguish between them ?

4. Whatever you do: would caching be an option?

Precalculation is often a great weapon but this time I can't see a viable way to implement it.

So ...

The third idea is the one I like the most.
I initially discarded it but, since you talked about it, maybe it is wise to reconsider it. What do you think ?

If you want to take a look at what I'm doing, here's the link: http://www.nkgcommunity.it/VanillaTest/
I've temporarily switched from Italian to English language for you.
Please, use the mobile version because it's a bit more refined. Note that this is only a test web site and that there's a lot of garbage.
Try to read a discussion and you'll see the integration between the gallery and the discussions. In this version the images are only displayed at the top of the page and not between comments.
In the menu you'll find the gallery under the "AlbGallery" menu item. There you'll see a main album for each user. Inside that album the users can create other albums.

Thanks again.

--Alberto

alblon · May 2017

Ok, I tested that solution and it works !
I used the "BeforeGet" event to change the $Limit and $Offset parameters of the getByDiscussion method and the "AfterGet" event to get the additional comments and to remove them from the result set.
All seems to work flawlessly.
To distinguish between the getByDiscussion , getByUserand getByUser2methods I simply checked the presence of the 'DiscussionID' argument: it is only present if the event is fired by the getByDiscussion.

rbrahmson · May 2017

Hi @albion, read your well described question and reply to @R_J's always useful suggestions. You may already have thought about it, but I wonder where you keep the photos and where they are indexed so you know where to insert them between the comments. What about adding a comment any time a user adds photos and have the comment body link to the photos and perhaps include some other info you save in a newly created field (to distinguish from user written comments)? Then you can just change what you show based upon the content of the comment (user written or your automatically inserted comment) and bypass the entire problem you were trying to solve?

R_J · May 2017

Thanks for the insights! Am I right that
a) you have a table for comments (GDN_Comment) and
b) a table for the album images (GDN_Media?)

In a discussion you want to show comments and images sorted by date. You fetch all images and then try to inject them. That would be an explanation for what you try to do.

Is that correct?

R_J · May 2017

Given that this is correct, I guess you have an AlbumID info in the GDN_Media which is connected to a AlbumID in the GDN_Discussion table.

What I guess would be a good solution to your problem is: whenever the album user saves a new picture, you are able to get the connected Discussion. In that Discussion there is the field LastCommentID.

If you save that CommentID to the GDN_Media table as "ShowAfterCommentID", you have the info you need.

alblon · June 2017

Hi R_J,
yes you are right with your guess, you nearly read my mind about the details of my implementation.
Unfortunately it seems to me that your idea do not solve all of my problems. What if I have to display images at the start of a page that is not the first one ? In that case I don't want to display images at the end of the previous page because it's ugly, but at the start of the new page. To do that I have to know the ID of the last comment of the previous page and I fall back to the previous problem: reading the comment just before the first one of the page.
I can't also write down the ID of the next comment of the image, because it doesn't exist yet.

However thank you very much for your efforts in helping me. And indeed you've helped me because you made me rethink about the getByDiscussion method. Changing the limit and offset works perfectly.

Thank you again.

alblon · June 2017

@rbrahmson said:
... What about adding a comment any time a user adds photos and have the comment body link to the photos and perhaps include some other info you save in a newly created field (to distinguish from user written comments)? Then you can just change what you show based upon the content of the comment (user written or your automatically inserted comment) and bypass the entire problem you were trying to solve?

Yes, I thought about having a comment tied to an image. Unfortunately it breaks the paging because each page would have a different number of comments depending on how many images are there.
Thank you anyway !

R_J · June 2017

I feel uneasy because from all alternatives that is the most "dirty" one... But I understand it is working for you and it offers the solution you were striving for right from the beginning.

There is only one thing I'm wondering when reading your description. Say you have a discussion with 20 comments and you display 20 comments per page. And there should be one album connected to that discussion with some pictures having a later DateInserted than the last comment.
In such a case those images would be at the very end of the discussion.

When someone comments on that discussion, that would be the 21st comment and the discussion now would show two pages.

Is it correct that those pictures which had been at the bottom of a one page discussion now would change to be pictures shown at the top of the second page?
I think your users wouldn't have too big problems with that, but to me that sounds illogical and I would try to avoid that in order not to confuse people.

I, as a user, would expect to find some content which I saw on page X always on page X and not suddenly on page X + 1.

alblon · June 2017

Hi R_J, as a professional programmer (but in a different field, I'm not a web developer) I agree with you and I found that solution dirty.
But this is an application done to support an hobby forum, done in my little spare time. So I have to make decisions that I don't fully like. But I don't fully like any other alternative, so I prefer the fastest one, provided that it doesn't jeopardize the stability of the forum and my ability to mantain it.

The choice to show the new images at the start of the next page is desirable for the kind of forum I'm building.

Thank you again, it's awsome to receive this kind of support on a forum !

How can I get the next or previous comment in a page ?

Best Answer

1. Database caching

2. Simple SQL

3. Change the initial query to include your results

4. Whatever you do: would caching be an option?

5. Why? ;-)

Answers

1. Database caching

2. Simple SQL

3. Change the initial query to include your results

4. Whatever you do: would caching be an option?

5. Why? ;-)