Use AR relations when merging comments

Using arrays made it difficult to order by more than one field (like the
`most_voted` scope does), and so we were ordering by `confidence_score`
and ignoring the `created_at` column.

Using an AR relation makes it easy to reuse the existing `most_voted`
scope.

This change has one side effect: now comments with equal votes are
ordered in descending order instead of having no specific order. That
means flaky specs which failed sometimes because they assumed comments
were ordered by date are now always green.

I've also re-added the `oldest` scope removed in 792b15b thinking it was
removed because using it with arrays was too hard.
This commit is contained in:
Javi Martín
2018-08-24 21:50:58 +02:00
parent 7b5ce80593
commit dbcc5fb724
3 changed files with 7 additions and 18 deletions

View File

@@ -8,7 +8,7 @@ class Legislation::AnnotationsController < Legislation::BaseController
load_and_authorize_resource :draft_version, through: :process load_and_authorize_resource :draft_version, through: :process
load_and_authorize_resource load_and_authorize_resource
has_orders %w{most_voted newest}, only: :show has_orders %w[most_voted newest oldest], only: :show
def index def index
@annotations = @draft_version.annotations @annotations = @draft_version.annotations

View File

@@ -1,24 +1,13 @@
class MergedCommentTree < CommentTree class MergedCommentTree < CommentTree
attr_reader :commentables, :array_order attr_reader :commentables
def initialize(commentables, page, order = "confidence_score") def initialize(commentables, page, order = "confidence_score")
@commentables = commentables @commentables = commentables
super(commentables.first, page, order) super(commentables.first, page, order)
@array_order = set_array_order(order)
end end
def root_comments def base_comments
Kaminari.paginate_array(commentables.flatten.map(&:comments).map(&:roots).flatten.sort_by {|a| a[array_order]}.reverse). Comment.where(commentable: commentables.flatten)
page(page).per(ROOT_COMMENTS_PER_PAGE)
end
def set_array_order(order)
case order
when "newest"
"created_at"
else
"confidence_score"
end
end end
end end

View File

@@ -14,7 +14,7 @@ describe "Commenting legislation questions" do
expect(page).to have_css(".comment", count: 4) expect(page).to have_css(".comment", count: 4)
comment = Comment.first comment = Comment.last
within first(".comment") do within first(".comment") do
expect(page).to have_content comment.user.name expect(page).to have_content comment.user.name
expect(page).to have_content I18n.l(comment.created_at, format: :datetime) expect(page).to have_content I18n.l(comment.created_at, format: :datetime)
@@ -144,7 +144,7 @@ describe "Commenting legislation questions" do
legislation_annotation.draft_version, legislation_annotation.draft_version,
legislation_annotation) legislation_annotation)
within all(".comment").last do within all(".comment").first do
expect(page).to have_content "Built with http://rubyonrails.org/" expect(page).to have_content "Built with http://rubyonrails.org/"
expect(page).to have_link("http://rubyonrails.org/", href: "http://rubyonrails.org/") expect(page).to have_link("http://rubyonrails.org/", href: "http://rubyonrails.org/")
expect(find_link("http://rubyonrails.org/")[:rel]).to eq("nofollow") expect(find_link("http://rubyonrails.org/")[:rel]).to eq("nofollow")
@@ -160,7 +160,7 @@ describe "Commenting legislation questions" do
legislation_annotation.draft_version, legislation_annotation.draft_version,
legislation_annotation) legislation_annotation)
within all(".comment").last do within all(".comment").first do
expect(page).to have_content "click me http://www.url.com" expect(page).to have_content "click me http://www.url.com"
expect(page).to have_link("http://www.url.com", href: "http://www.url.com") expect(page).to have_link("http://www.url.com", href: "http://www.url.com")
expect(page).not_to have_link("click me") expect(page).not_to have_link("click me")