Skip to content
This repository has been archived by the owner on Feb 5, 2018. It is now read-only.

students_team action #41

Open
wants to merge 2 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
6 changes: 3 additions & 3 deletions lib/teachers_pet/actions/base.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

require 'configuration'
require 'octokit'
require 'highline/import'

## Common code for the edugit scripts.
module TeachersPet
Expand Down Expand Up @@ -113,11 +114,11 @@ def get_team_member_logins(team_id)
end

def read_file(filename, type)
# Return a hash with team name as key and list of member(s) as value.
Copy link
Contributor

Choose a reason for hiding this comment

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

Minor: mind moving this comment one line up, above the method definition?

map = Hash.new
puts "Loading #{type}:"
File.open(filename).each_line do |team|
# Team can be a single user, or a team name and multiple users
# Trim whitespace, otherwise issues occur
# Each line is a single user, or a team name and multiple users
team.strip!
items = team.split(' ')
items.each do |item|
Expand All @@ -143,7 +144,6 @@ def read_file(filename, type)
end

def confirm(message, abort_on_no = true)
# confirm
confirmed = false
choose do |menu|
menu.prompt = message
Expand Down
1 change: 0 additions & 1 deletion lib/teachers_pet/actions/clone_repos.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@

require 'rubygems'
require 'highline/question'
require 'highline/import'
require 'highline/compatibility'
require 'teachers_pet/actions/base'

Expand Down
1 change: 0 additions & 1 deletion lib/teachers_pet/actions/create_repos.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@

require 'rubygems'
require 'highline/question'
require 'highline/import'
require 'highline/compatibility'
require 'teachers_pet/actions/base'

Expand Down
1 change: 0 additions & 1 deletion lib/teachers_pet/actions/create_teams.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@

require 'rubygems'
require 'highline/question'
require 'highline/import'
require 'highline/compatibility'

require 'teachers_pet/actions/base'
Expand Down
1 change: 0 additions & 1 deletion lib/teachers_pet/actions/fork_collab.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

require 'rubygems'
require 'highline/question'
require 'highline/import'
require 'highline/compatibility'
require 'teachers_pet/actions/base'

Expand Down
1 change: 0 additions & 1 deletion lib/teachers_pet/actions/open_issue.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@

require 'rubygems'
require 'highline/question'
require 'highline/import'
require 'teachers_pet/actions/base'

module TeachersPet
Expand Down
1 change: 0 additions & 1 deletion lib/teachers_pet/actions/push_files.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

require 'rubygems'
require 'highline/question'
require 'highline/import'
require 'highline/compatibility'
require 'teachers_pet/actions/base'

Expand Down
79 changes: 79 additions & 0 deletions lib/teachers_pet/actions/students_team.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
# Create a team from all users named in a students file

$LOAD_PATH << File.join(File.dirname(__FILE__), '..', '..')

require 'set'
require 'rubygems'
require 'highline/question'
require 'highline/compatibility'

require 'teachers_pet/actions/base'

module TeachersPet
module Actions
class StudentsTeam < Base
Copy link
Contributor

Choose a reason for hiding this comment

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

Just to be more explicit, I'd call this class/file CreateStudentsTeam.

def read_info
@organization = ask("What is the organization name?") { |q| q.default = TeachersPet::Configuration.organization }
@student_file = self.get_students_file_path
end

def load_files
@students = read_file(@student_file, 'Students')
end

def update_team
self.init_client

all_team_name='Students'
confirm("Add #{@students.size} members to '#{all_team_name}' team?")

# Get current members of org team
existing = Hash.new
Copy link
Contributor

Choose a reason for hiding this comment

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

Not actually used...

teams = self.get_teams_by_name(@organization)
studentsteam = teams[all_team_name] || false
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's change this line to

student_team_hash = teams[all_team_name]

Since teams is a Hash, the value will be nil if the team doesn't exist.

unless studentsteam
puts "\nCreating team #{all_team_name}..."
@client.create_team(@organization,
{
:name => all_team_name,
:permission => 'push'
})
studentsteam = self.get_teams_by_name(@organizaiton)['Students']
end

# Find and add new students to the org team
all_students = Set.new
@students.each do |team, members|
all_students.merge(members)
end
all_team_members = Set.new(self.get_team_member_logins(studentsteam[:id]))
new_students = all_students - all_team_members
old_students = all_team_members - all_students
Copy link
Contributor

Choose a reason for hiding this comment

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

Thinking we could make these variable names more clear, e.g. usernames_to_add, existing_members, etc.


puts "\nMembers of '#{studentsteam[:name]}':"
all_team_members.each do |member|
puts " -> #{member}"
end
puts "\nAdding members:"
new_students.each do |student|
@client.add_team_member(studentsteam[:id], student)
puts " -> #{student}"
end
puts "\nMembers not mentioned:"
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this need to be more clear?

Yeah, maybe rephrasing to something like "Existing members of the team, who weren't listed in the students file".

old_students.each do |student|
puts " -> #{student}"
end

puts "\n#{all_team_members.length} previous members in '#{studentsteam[:name]}'."
puts "#{new_students.length} members added."
puts "#{(all_students - new_students).length} skipped."
end
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we break this method down into smaller pieces?


def run
self.read_info
self.load_files
self.update_team
end
end
end
end