Extend methods for retrieving next/previous items with conditions

[xiaohui-zhangxh]

There is a scenario where all blogs are positioned without any scope. In the frontend view, we need to retrieve the next/previous published blog. I hope this Pull Request can be accepted.

class Blog < ActiveRecord::Base
  acts_as_list
  scope :published, -> { where(published: true) }
end

class BlogsController < ApplicationController
  def show
    @blog = Blog.published.find(params[:id])
    @next_blog = @blog.lower_item(published: true) # next published blog
    @prev_blog = @blog.higher_item(published: true) # previous published blog
  end
end

class Admin::BlogsController < ApplicationController
  def show
    @blog = Blog.find(params[:id])
    @next_blog = @blog.lower_item # next blog
    @prev_blog = @blog.higher_item # previous blog
  end
end

[brendon]

Hi @xiaohui-zhangxh, thank you for this pull request. I think it can be solved more simply than this however.

Have you tried @blog.lower_items.published.first (or last) depending on what you want from the list.

I don't think there's a need to add this feature to acts_as_list specifically as it's fairly readable to use association methods here in your own code.

I'm happy for you to try and convince me otherwise :D

[xiaohui-zhangxh]

Hi @xiaohui-zhangxh, thank you for this pull request. I think it can be solved more simply than this however.

Have you tried @blog.lower_items.published.first (or last) depending on what you want from the list.

I don't think there's a need to add this feature to acts_as_list specifically as it's fairly readable to use association methods here in your own code.

I'm happy for you to try and convince me otherwise :D

Hi @brendon,

Yes, I have tried using @blog.lower_items.published.first. To maintain the same logic as lower_item, the code would look something like this:

@next_blog = @blog.lower_items.published.first if @blog.in_list?

While this approach is indeed straightforward, I believe that using @blog.lower_item or @blog.lower_item(published: true) offers a more elegant and concise solution.

I'm open to your thoughts on this.

[brendon]

I agree, it's slightly more elegant, but the problem is it's really outside the scope of acts_as_list's responsibilities. We give you the methods to manage the list but your published scope is external to that so you need to manually add that in as a condition. I'm not very keen on catering for passing conditions to all of our methods where that could make sense given these things can be chained to those relations / queries quite easily.

You could perhaps add some convenience methods on your model to return these items that are specific to your use case?

[xiaohui-zhangxh]

I agree, it's slightly more elegant, but the problem is it's really outside the scope of acts_as_list's responsibilities. We give you the methods to manage the list but your published scope is external to that so you need to manually add that in as a condition. I'm not very keen on catering for passing conditions to all of our methods where that could make sense given these things can be chained to those relations / queries quite easily.

You could perhaps add some convenience methods on your model to return these items that are specific to your use case?

You're right, I can simply add our own methods:

class Blog
  def prev_published_blog
    higher_items.published.last if in_list?
  end

  def next_published_blog
    lower_items.published.first if in_list?
  end

Thanks for your quick response. This PR can be closed.

[brendon]

Excellent :) And you're most welcome. I'm glad you were able to come up with something that worked for you.

I think in_list? shouldn't be required if you're not explicitly setting nil positions since we always position an item in the list on create.