Skip to content

Commit

Permalink
Fetch oldest authored_at correctly instead of oldest authored_at per …
Browse files Browse the repository at this point in the history
…page

Fixes rubygems#4448

Previously, the "since date" was using oldest `version.authored_at` for a given
page. This change fixes it to get the oldest `version.authored_at` in
the database. Because `authored_at` depends on either `built_at` or
`created_at`, I fetch both and take the min. This should only be 2
additional queries of 1 record each.

I modified existing tests to check for the "versions since" text and
also have 2 pages of actual content.
  • Loading branch information
albertchae committed Feb 20, 2024
1 parent 1d33ca5 commit ade3eaf
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 13 deletions.
9 changes: 9 additions & 0 deletions app/controllers/versions_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ class VersionsController < ApplicationController

def index
set_page
set_oldest_version_date
@versions = @rubygem.versions.by_position.page(@page).per(Gemcutter::VERSIONS_PER_PAGE)
end

Expand All @@ -14,4 +15,12 @@ def show
@on_version_page = true
render "rubygems/show"
end

private

def set_oldest_version_date
oldest_created_at = @rubygem.versions.order(:created_at).first
oldest_built_at = @rubygem.versions.order(:built_at).first
@oldest_version_date = [oldest_created_at, oldest_built_at].compact.map(&:authored_at).min
end
end
2 changes: 1 addition & 1 deletion app/views/versions/index.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
<p><%= t('.not_hosted_notice') %></p>
</div>
<% else %>
<h3 class="t-list__heading"><%= t('.versions_since', :count => @versions.total_count, :since => nice_date_for(@versions.map(&:authored_at).min)) %>:</h3>
<h3 class="t-list__heading"><%= t('.versions_since', :count => @versions.total_count, :since => nice_date_for(@oldest_version_date)) %>:</h3>
<div class="versions">
<ul class="t-list__items">
<%= render @versions %>
Expand Down
34 changes: 22 additions & 12 deletions test/functional/versions_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -114,27 +114,37 @@ class VersionsControllerTest < ActionController::TestCase
end

assert_select ".gem__version__date sup", text: "*", count: 1

assert_select ".t-list__heading", text: /1 version since January 01, 2000/, count: 1
end
end

context "on GET to index" do
setup do
@rubygem = create(:rubygem)
create(:version, number: "1.1.2", rubygem: @rubygem)
@oldest_created_at = Date.parse("2010-01-01")
create(:version, number: "1.1.2", rubygem: @rubygem, position: 0)
create(:version, number: "1.1.1", rubygem: @rubygem, position: 1, created_at: @oldest_created_at)
end

should "get paginated result" do
# first page includes the only version
get :index, params: { rubygem_id: @rubygem.name }

assert_response :success
assert page.has_content?("1.1.2")

# second page does not include the only version
get :index, params: { rubygem_id: @rubygem.name, page: 2 }

assert_response :success
refute page.has_content?("1.1.2")
stub_const(Gemcutter, :VERSIONS_PER_PAGE, 1) do
# first page only includes the version at position 0
get :index, params: { rubygem_id: @rubygem.name }

assert_response :success
assert page.has_content?("1.1.2")
refute page.has_content?("1.1.1")
assert_select ".t-list__heading", text: /2 versions since January 01, 2010/, count: 1

# second page only includes the version at position 1
get :index, params: { rubygem_id: @rubygem.name, page: 2 }

assert_response :success
refute page.has_content?("1.1.2")
assert page.has_content?("1.1.1")
assert_select ".t-list__heading", text: /2 versions since January 01, 2010/, count: 1
end
end
end

Expand Down

0 comments on commit ade3eaf

Please sign in to comment.