Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fetch oldest authored_at correctly instead of oldest authored_at per page #4460

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 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
@oldest_version_date = @rubygem.versions.oldest_authored_at
@versions = @rubygem.versions.by_position.page(@page).per(Gemcutter::VERSIONS_PER_PAGE)
end

Expand Down
13 changes: 13 additions & 0 deletions app/models/version.rb
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,19 @@ def rely_on_built_at?
built_at && built_at <= RUBYGEMS_IMPORT_DATE
end

def self.oldest_authored_at
minimum(
<<~SQL.squish
CASE WHEN DATE(created_at) = '#{RUBYGEMS_IMPORT_DATE}'
AND built_at <= created_at THEN
built_at
ELSE
created_at
END
SQL
)
end

def refresh_rubygem_indexed
rubygem.refresh_indexed!
end
Expand Down
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
19 changes: 19 additions & 0 deletions test/models/version_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,25 @@ class VersionTest < ActiveSupport::TestCase
end
end

context ".oldest_authored_at scope" do
setup do
@built_at = Version::RUBYGEMS_IMPORT_DATE - 60.days
@version.update(built_at: @built_at, created_at: Version::RUBYGEMS_IMPORT_DATE)
_newer_version = create(:version, rubygem: @version.rubygem)
end

should "return the built_at of the oldest version when created_at is the same as RUBYGEMS_IMPORT_DATE" do
assert_equal @built_at, Version.oldest_authored_at
end

should "return the created_at of the oldest version when created_at is not the same as RUBYGEMS_IMPORT_DATE" do
created_at = Version::RUBYGEMS_IMPORT_DATE + 1.day
@version.update(created_at: created_at)

assert_equal created_at, Version.oldest_authored_at
end
end

should "have a rubygems version" do
@version.update(required_rubygems_version: @required_rubygems_version)
new_version = Version.find(@version.id)
Expand Down
Loading