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

Add 'Until' column to contributors index #303

Open
wants to merge 1 commit into
base: main
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
2 changes: 1 addition & 1 deletion app/assets/stylesheets/screen.scss
Original file line number Diff line number Diff line change
Expand Up @@ -330,7 +330,7 @@ TABLES GENERALS
font-size: 1.1em;
}

#table-wrap table td.contributor-since {
#table-wrap table td.contributor-timestamp {
white-space: nowrap;
text-align: left;
}
Expand Down
21 changes: 19 additions & 2 deletions app/models/contributor.rb
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,8 @@ def self._all_with_ncommits(joins, where=nil)
order('ncommits DESC, contributors.url_id ASC')
end

def self.set_first_contribution_timestamps(only_new)
scope = only_new ? 'first_contribution_at IS NULL' : '1 = 1'
def self.set_first_contribution_timestamps(only_missing)
scope = only_missing ? 'first_contribution_at IS NULL' : '1 = 1'

connection.execute(<<-SQL)
UPDATE contributors
Expand All @@ -56,6 +56,23 @@ def self.set_first_contribution_timestamps(only_new)
SQL
end

def self.set_last_contribution_timestamps(only_missing)
scope = only_missing ? 'last_contribution_at IS NULL' : '1 = 1'

connection.execute(<<-SQL)
UPDATE contributors
SET last_contribution_at = last_contributions.committer_date
FROM (
SELECT contributor_id, MAX(commits.committer_date) AS committer_date
FROM contributions
INNER JOIN commits ON commits.id = commit_id
GROUP BY contributor_id
) AS last_contributions
WHERE id = last_contributions.contributor_id
AND #{scope}
SQL
end

# The contributors table may change if new name equivalences are added and IDs
# in particular are expected to move. So, we just put the parameterized name
# in URLs, which is unique anyway.
Expand Down
5 changes: 3 additions & 2 deletions app/models/repo.rb
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ def sync
if ncommits > 0 || nreleases > 0 || rebuild_all
sync_names
sync_ranks
sync_first_contribution_timestamps
sync_contribution_timestamps
end

RepoUpdate.create!(
Expand Down Expand Up @@ -188,8 +188,9 @@ def sync_ranks
end
end

def sync_first_contribution_timestamps
def sync_contribution_timestamps
Contributor.set_first_contribution_timestamps(!rebuild_all)
Contributor.set_last_contribution_timestamps(!rebuild_all)
end

# Determines whether the names mapping has been updated. This is useful because
Expand Down
5 changes: 3 additions & 2 deletions app/views/contributors/_contributor.html.erb
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
<tr class="<%= cycle 'even', 'odd' %>">
<tr id="<%= contributor.name.downcase.tr(' ', '-') %>" class="<%= cycle 'even', 'odd' %>">
<td class="contributor-rank">#<%= contributor.rank %></td>
<td class="contributor-name"><%= link_to_contributor contributor %></td>
<td class="contributor-since"><%= date contributor.first_contribution_at %></td>
<td class="contributor-timestamp"><%= date contributor.first_contribution_at %></td>
<td class="contributor-timestamp"><%= date contributor.last_contribution_at %></td>
<td class="no-commits">
<% path = if @time_window
contributor_commits_in_time_window_path(contributor, @time_window)
Expand Down
1 change: 1 addition & 0 deletions app/views/contributors/index.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
<th></th>
<th>Name</th>
<th>Since</th>
<th>Until</th>
<th>Commits</th>
</tr>
<%= render @contributors %>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
class AddFirstContributionAtToContributors < ActiveRecord::Migration[4.2]
def up
add_column :contributors, :first_contribution_at, :datetime
Contributor.try(:fill_missing_first_contribution_timestamps)
Contributor.set_first_contribution_timestamps
end

def down
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
class AddLastContributionAtToContributors < ActiveRecord::Migration[7.1]
def up
add_column :contributors, :last_contribution_at, :datetime
Contributor.set_last_contribution_timestamps
end

def down
remove_column :contributors, :last_contribution_at
end
end
3 changes: 2 additions & 1 deletion db/schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
#
# It's strongly recommended that you check this file into your version control system.

ActiveRecord::Schema[7.0].define(version: 2016_05_12_095609) do
ActiveRecord::Schema[7.1].define(version: 2024_08_24_030051) do
# These are extensions that must be enabled in order to support this database
enable_extension "plpgsql"

Expand Down Expand Up @@ -42,6 +42,7 @@
t.string "url_id", null: false
t.integer "rank"
t.datetime "first_contribution_at", precision: nil
t.datetime "last_contribution_at"
t.index ["name"], name: "index_contributors_on_name", unique: true
t.index ["url_id"], name: "index_contributors_on_url_id", unique: true
end
Expand Down
2 changes: 1 addition & 1 deletion test/controllers/commits_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ def test_index_for_contributors
cases = [
[:david, [:commit_339e4e8, :commit_e0ef631]],
[:jeremy, [:commit_b821094, :commit_7cdfd91, :commit_5b90635]],
[:jose, [:commit_5b90635]],
[:jose, [:commit_e243a8a, :commit_5b90635]],
].map {|a, b| [contributors(a), Array(commits(*b))]}

cases.each do |contributor, commits|
Expand Down
22 changes: 18 additions & 4 deletions test/controllers/contributors_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
class ContributorsControllerTest < ActionController::TestCase
def test_index_main_listing
# Order by ncommits DESC, url_id ASC.
expected = [[:jeremy, 3], [:david, 2], [:jose, 1], [:vijay, 1], [:xavier, 1]]
expected = [[:jeremy, 3], [:david, 2], [:jose, 2], [:vijay, 1], [:xavier, 1]]

get :index

Expand All @@ -13,13 +13,27 @@ def test_index_main_listing
assert_equal expected.size, actual.size

expected.zip(actual).each do |e, a|
assert_equal contributors(e.first).name, a.name
contributor = contributors(e.first)

assert_equal contributor.name, a.name
assert_equal e.second, a.ncommits

assert_select "tr##{contributor.name.downcase.tr(' ', '-') }" do |elements|
assert_select 'td.contributor-rank', "##{contributor.rank.to_s}"
assert_select 'td.contributor-name', /#{contributor.name}/
assert_select 'td.contributor-timestamp' do |elements|
assert_equal 2, elements.size
assert_equal contributor.first_contribution_at.strftime("%d %b %Y"), elements[0].text.strip
assert_equal contributor.last_contribution_at.strftime("%d %b %Y"), elements[1].text.strip
end
assert_select "td.no-commits", e.second.to_s
end
end
end

def test_index_by_release
releases = {
'v4.0.0' => [[:jose, 1]],
'v3.2.0' => [[:jeremy, 1], [:jose, 1], [:vijay, 1]],
'v0.14.4' => [[:david, 1]]
}
Expand Down Expand Up @@ -48,11 +62,11 @@ def test_in_time_window
date_range = '20121201-20121231'

time_windows = {
'all-time' => [[:jeremy, 3], [:david, 2], [:jose, 1], [:vijay, 1], [:xavier, 1]],
'all-time' => [[:jeremy, 3], [:david, 2], [:jose, 2], [:vijay, 1], [:xavier, 1]],
'today' => [[:jeremy, 1]],
'this-week' => [[:jeremy, 1], [:xavier, 1]],
'this-month' => [[:david, 1], [:jeremy, 1], [:xavier, 1]],
'this-year' => [[:jeremy, 3], [:david, 1], [:jose, 1], [:vijay, 1], [:xavier, 1]],
'this-year' => [[:jeremy, 3], [:jose, 2], [:david, 1], [:vijay, 1], [:xavier, 1]],
since => [[:jeremy, 1], [:xavier, 1]],
date_range => [[:david, 1], [:jeremy, 1], [:xavier, 1]],
}
Expand Down
5 changes: 2 additions & 3 deletions test/controllers/releases_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,8 @@ def test_index
get :index
assert_response :success

assert_select 'span.listing-total', 'Showing 5 releases'
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Duplicated below.


expected = %w(
v4_0_0
v3_2_0
v2_3_2_1
v2_3_2
Expand All @@ -24,7 +23,7 @@ def test_index
assert_equal e.contributors.count, a.ncontributors
end

assert_select 'span.listing-total', 'Showing 5 releases'
assert_select 'span.listing-total', "Showing #{expected.size} releases"
assert_select 'li.current', 'Releases'
end
end
20 changes: 17 additions & 3 deletions test/fixtures/commits.yml
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# WARNING: contributors.yml has first contribution timestamps that depend on
# WARNING: contributors.yml has contribution timestamps that depend on
# these fixtures. Keep them in sync as needed.
#
# In the following comments we assumed today is 26 December 2012.
Expand Down Expand Up @@ -40,7 +40,7 @@ commit_26c024e:
one, because it is issued at parse-time. It seemed to in
some places because the constant was the only expression in
the block and therefore it was its return value, that could
potentially be used by silence_warnings are return value of
potentially be used by silence_warnings as a return value of
the yield call.

To bypass the warning we assign to a variable. The chosen
Expand Down Expand Up @@ -90,6 +90,20 @@ commit_5b90635:
release: 'v3_2_0'
merge: false

# This commit belongs to this year, release 4.0.0
commit_e243a8a:
sha1: 'e243a8a32eb4c8777f07ca4b974bd7e38d9477d3'
author_email: '[email protected]'
author_name: 'José Valim'
author_date: '2012-07-18 07:21:57'
committer_email: '[email protected]'
committer_name: 'José Valim'
committer_date: '2012-07-18 07:47:52'
message: |
Update changelog for migration generator change
release: v4_0_0
merge: false

# This commit is seven years old, release v0.14.4.
commit_e0ef631:
sha1: 'e0ef63105538f8d97faa095234f069913dd5229c'
Expand All @@ -106,7 +120,7 @@ commit_e0ef631:
release: 'v0_14_4'
merge: false

# This commit has a committer_date that is more recent the author_date and
# This commit has a committer_date that is more recent than the author_date and
# should appear in the contributions list before 5b90635.
commit_7cdfd91:
sha1: '7cdfd910b7cdd398f8b54542c5a6a17966a5c8f3'
Expand Down
4 changes: 4 additions & 0 deletions test/fixtures/contributions.yml
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,10 @@ contribution_<%= n += 1 %>:
commit: commit_5b90635
contributor: jose

contribution_<%= n += 1 %>:
commit: commit_e243a8a
contributor: jose

contribution_<%= n += 1 %>:
commit: commit_5b90635
contributor: jeremy
Expand Down
5 changes: 5 additions & 0 deletions test/fixtures/contributors.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,23 +2,28 @@ david:
name: 'David Heinemeier Hansson'
url_id: 'david-heinemeier-hansson'
first_contribution_at: '2005-11-11 10:07:24'
last_contribution_at: '2012-12-07 20:38:53'

jeremy:
name: 'Jeremy Daer'
url_id: 'jeremy-kemper'
first_contribution_at: '2012-01-19 19:49:13'
last_contribution_at: '2012-12-26 21:16:05'

jose:
name: 'José Valim'
url_id: 'jose-valim'
first_contribution_at: '2012-01-19 19:49:13'
last_contribution_at: '2012-07-18 07:47:52'

xavier:
name: 'Xavier Noria'
url_id: 'xavier-noria'
first_contribution_at: '2012-12-24 21:16:16'
last_contribution_at: '2012-12-24 21:16:16'

vijay:
name: 'Vijay Dev'
url_id: 'vijay-dev'
first_contribution_at: '2012-01-20 00:47:14'
last_contribution_at: '2012-01-20 00:47:14'
8 changes: 8 additions & 0 deletions test/fixtures/releases.yml
Original file line number Diff line number Diff line change
@@ -1,3 +1,11 @@
v4_0_0:
tag: 'v4.0.0'
major: 4
minor: 0
tiny: 0
patch: 0
date: '2013-06-25 14:27:04'

v3_2_0:
tag: 'v3.2.0'
major: 3
Expand Down
40 changes: 33 additions & 7 deletions test/models/repo_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,34 +4,53 @@ class RepoTest < ActiveSupport::TestCase
test '#sync_ranks' do
Repo.new.send(:sync_ranks)

{jeremy: 1, david: 2, jose: 3, xavier: 3, vijay: 3}.each do |c, r|
{jeremy: 1, david: 2, jose: 2, xavier: 4, vijay: 4}.each do |c, r|
Copy link
Author

@joshuay03 joshuay03 Aug 24, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems like a bug. Not introduced here cause it can be observed on the official site.

I think it makes more sense that if multiple contributors share a rank the next contributor should be assigned the next rank i.e. this should be:

Suggested change
{jeremy: 1, david: 2, jose: 2, xavier: 4, vijay: 4}.each do |c, r|
{jeremy: 1, david: 2, jose: 2, xavier: 3, vijay: 3}.each do |c, r|

assert_equal r, contributors(c).rank
end
end

test '#sync_first_contributed_timestamps' do
Contributor.update_all(first_contribution_at: nil)
Repo.new.send(:sync_first_contribution_timestamps)
test '#sync_contribution_timestamps' do
Contributor.update_all(first_contribution_at: nil, last_contribution_at: nil)
Repo.new.send(:sync_contribution_timestamps)

assert_first_contribution :commit_e0ef631, :david
assert_last_contribution :commit_339e4e8, :david

assert_first_contribution :commit_5b90635, :jeremy
assert_last_contribution :commit_b821094, :jeremy

assert_first_contribution :commit_5b90635, :jose
assert_last_contribution :commit_e243a8a, :jose

assert_first_contribution :commit_26c024e, :xavier
assert_last_contribution :commit_26c024e, :xavier

assert_first_contribution :commit_6c65676, :vijay
assert_last_contribution :commit_6c65676, :vijay
end

test '#sync_first_contributed_timestamps rebuilding all' do
test '#sync_contribution_timestamps rebuilding all' do
Contributor.update_all(
first_contribution_at: Commit.minimum(:committer_date) - 1.year
first_contribution_at: Commit.minimum(:committer_date) - 1.year,
last_contribution_at: Commit.maximum(:committer_date) + 1.year
)

Repo.new(rebuild_all: true).send(:sync_first_contribution_timestamps)
Repo.new(rebuild_all: true).send(:sync_contribution_timestamps)

assert_first_contribution :commit_e0ef631, :david
assert_last_contribution :commit_339e4e8, :david

assert_first_contribution :commit_5b90635, :jeremy
assert_last_contribution :commit_b821094, :jeremy

assert_first_contribution :commit_5b90635, :jose
assert_last_contribution :commit_e243a8a, :jose

assert_first_contribution :commit_26c024e, :xavier
assert_last_contribution :commit_26c024e, :xavier

assert_first_contribution :commit_6c65676, :vijay
assert_last_contribution :commit_6c65676, :vijay
end

def assert_first_contribution(commit, contributor)
Expand All @@ -40,4 +59,11 @@ def assert_first_contribution(commit, contributor)

assert_equal expected, actual
end

def assert_last_contribution(commit, contributor)
expected = commits(commit).committer_date
actual = contributors(contributor).last_contribution_at

assert_equal expected, actual
end
end
Loading