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

Make user_id foreign key for github team members #7027

Merged
merged 11 commits into from
Aug 14, 2024
Prev Previous commit
Next Next commit
Simplify create and destroy
  • Loading branch information
ErikSchierboom committed Aug 13, 2024
commit ca0f1454f4741e7d1766fc3af97339e139313add
10 changes: 6 additions & 4 deletions app/commands/github/team_member/create.rb
Original file line number Diff line number Diff line change
@@ -1,14 +1,16 @@
class Github::TeamMember::Create
include Mandate

initialize_with :user_id, :team_name
initialize_with :github_uid, :team_name
Copy link
Member Author

Choose a reason for hiding this comment

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

I've made it explicit that this is the GitHub UID.


def call
::Github::TeamMember.find_create_or_find_by!(user_id:, team_name:).tap do
User::UpdateMaintainer.(user) if user
return unless user

user.github_team_memberships.find_or_create_by!(team_name:).tap do
User::UpdateMaintainer.(user)
end
end

memoize
def user = User.find_by(uid: user_id)
def user = User.find_by(uid: github_uid)
end
10 changes: 6 additions & 4 deletions app/commands/github/team_member/destroy.rb
Original file line number Diff line number Diff line change
@@ -1,14 +1,16 @@
class Github::TeamMember::Destroy
include Mandate

initialize_with :user_id, :team_name
initialize_with :github_uid, :team_name

def call
::Github::TeamMember.where(user_id:, team_name:).destroy_all.tap do
User::UpdateMaintainer.(user) if user
return unless user

user.github_team_memberships.where(team_name:).delete_all.tap do
User::UpdateMaintainer.(user)
end
end

memoize
def user = User.find_by(uid: user_id)
def user = User.find_by(uid: github_uid)
end
4 changes: 2 additions & 2 deletions app/commands/github/team_member/sync_members.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ def call
private
def add_team_members!
org_team_members.each do |team_name, user_ids|
user_ids.each do |user_id|
::Github::TeamMember::Create.(user_id, team_name)
user_ids.each do |github_uid|
::Github::TeamMember::Create.(github_uid, team_name)
end
end
end
Expand Down
18 changes: 10 additions & 8 deletions test/commands/github/team_member/create_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,39 +2,41 @@

class Github::TeamMember::CreateTest < ActiveSupport::TestCase
test "creates team member" do
user_id = '137131'
github_uid = '137131'
team_name = 'fsharp'
user = create(:user, uid: github_uid)

team_name_member = Github::TeamMember::Create.(user_id, team_name)
team_name_member = Github::TeamMember::Create.(github_uid, team_name)

assert_equal 1, Github::TeamMember.count
assert_equal user_id, team_name_member.user_id
assert_equal user, team_name_member.user
assert_equal team_name, team_name_member.team_name
end

test "update maintainer role" do
user_id = '137131'
github_uid = '137131'
team_name = 'fsharp'

user = create(:user, uid: user_id)
user = create(:user, uid: github_uid)
User::UpdateMaintainer.expects(:call).with(user).once

team_name_member = Github::TeamMember::Create.(user_id, team_name)
team_name_member = Github::TeamMember::Create.(github_uid, team_name)

assert_equal 1, Github::TeamMember.count
assert_equal user_id, team_name_member.user_id
assert_equal user, team_name_member.user
assert_equal team_name, team_name_member.team_name
end

test "idempotent" do
user_id = '137131'
team_name = 'fsharp'
user = create(:user, uid: user_id)

assert_idempotent_command do
Github::TeamMember::Create.(user_id, team_name)
end

assert_equal 1, Github::TeamMember.count
assert Github::TeamMember.where(user_id:, team_name:).exists?
assert Github::TeamMember.where(user:, team_name:).exists?
end
end
27 changes: 15 additions & 12 deletions test/commands/github/team_member/destroy_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,39 +2,42 @@

class Github::TeamMember::DestroyTest < ActiveSupport::TestCase
test "removes team member" do
user_id = '137131'
github_uid = '137131'
team_name = 'fsharp'

create(:github_team_member, team_name:, user_id:)
user = create(:user, uid: github_uid)
create(:github_team_member, team_name:, user:)

# Sanity check
assert Github::TeamMember.where(user_id:, team_name:).exists?
assert Github::TeamMember.where(user:, team_name:).exists?

Github::TeamMember::Destroy.(user_id, team_name)
Github::TeamMember::Destroy.(github_uid, team_name)

refute Github::TeamMember.where(user_id:, team_name:).exists?
refute Github::TeamMember.where(user:, team_name:).exists?
end

test "update maintainer role" do
user_id = '137131'
github_uid = '137131'
team_name = 'fsharp'

create(:github_team_member, team_name:, user_id:)
user = create(:user, uid: github_uid)
create(:github_team_member, team_name:, user:)

user = create(:user, uid: user_id)
User::UpdateMaintainer.expects(:call).with(user).once

Github::TeamMember::Destroy.(user_id, team_name)
Github::TeamMember::Destroy.(github_uid, team_name)
end

test "idempotent" do
user_id = '137131'
github_uid = '137131'
team_name = 'fsharp'

user = create(:user, uid: github_uid)

assert_idempotent_command do
Github::TeamMember::Destroy.(user_id, team_name)
Github::TeamMember::Destroy.(github_uid, team_name)
end

refute Github::TeamMember.where(user_id:, team_name:).exists?
refute Github::TeamMember.where(user:, team_name:).exists?
end
end