FIX: Return next bookmarks page only if it exists (#18139)

It used to return the next URL anyway which lead to an additional
request. On the frontend, if the result set was empty, it kept retrying
until at least one result was returned. This bug is fixed in this commit
too.
This commit is contained in:
Bianca Nenciu 2022-09-01 13:04:00 +03:00 committed by GitHub
parent 07aa324f61
commit 5092c9804c
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 11 additions and 4 deletions

View File

@ -96,6 +96,7 @@ export default Controller.extend({
_processLoadResponse(searchTerm, response) { _processLoadResponse(searchTerm, response) {
if (!response || !response.user_bookmark_list) { if (!response || !response.user_bookmark_list) {
this.model.loadMoreUrl = null;
return; return;
} }

View File

@ -5,7 +5,7 @@ class UserBookmarkList
PER_PAGE = 20 PER_PAGE = 20
attr_reader :bookmarks, :per_page attr_reader :bookmarks, :per_page, :has_more
attr_accessor :more_bookmarks_url, :bookmark_serializer_opts attr_accessor :more_bookmarks_url, :bookmark_serializer_opts
def initialize(user:, guardian:, params:) def initialize(user:, guardian:, params:)
@ -21,7 +21,9 @@ class UserBookmarkList
end end
def load(&blk) def load(&blk)
@bookmarks = BookmarkQuery.new(user: @user, guardian: @guardian, params: @params).list_all(&blk) query = BookmarkQuery.new(user: @user, guardian: @guardian, params: @params)
@bookmarks = query.list_all(&blk)
@has_more = (@params[:page].to_i + 1) * @params[:per_page] < query.count
@bookmarks @bookmarks
end end

View File

@ -15,6 +15,6 @@ class UserBookmarkListSerializer < ApplicationSerializer
end end
def include_more_bookmarks_url? def include_more_bookmarks_url?
@include_more_bookmarks_url ||= object.bookmarks.size == object.per_page @include_more_bookmarks_url ||= object.has_more
end end
end end

View File

@ -26,7 +26,7 @@ class BookmarkQuery
end end
end end
attr_reader :guardian attr_reader :guardian, :count
def initialize(user:, guardian: nil, params: {}) def initialize(user:, guardian: nil, params: {})
@user = user @user = user
@ -34,6 +34,7 @@ class BookmarkQuery
@guardian = guardian || Guardian.new(@user) @guardian = guardian || Guardian.new(@user)
@page = @params[:page].to_i @page = @params[:page].to_i
@limit = @params[:limit].present? ? @params[:limit].to_i : @params[:per_page] @limit = @params[:limit].present? ? @params[:limit].to_i : @params[:per_page]
@count = 0
end end
def list_all(&blk) def list_all(&blk)
@ -73,6 +74,8 @@ class BookmarkQuery
bookmarks.updated_at DESC" bookmarks.updated_at DESC"
) )
@count = results.count
if @page.positive? if @page.positive?
results = results.offset(@page * @params[:per_page]) results = results.offset(@page * @params[:per_page])
end end

View File

@ -20,6 +20,7 @@ RSpec.describe UserBookmarkList do
it "returns all types of bookmarks" do it "returns all types of bookmarks" do
list.load list.load
expect(list.bookmarks.map(&:id)).to match_array([post_bookmark.id, topic_bookmark.id, user_bookmark.id]) expect(list.bookmarks.map(&:id)).to match_array([post_bookmark.id, topic_bookmark.id, user_bookmark.id])
expect(list.has_more).to eq(false)
end end
it "defaults to 20 per page" do it "defaults to 20 per page" do