Skip to content

Commit

Permalink
🔧 Sync Course managers to API server when creating course, #151
Browse files Browse the repository at this point in the history
    - Request to original API with HTTP POST/PUT/DELETE method for Course resource
  • Loading branch information
kyoshizaki committed Nov 26, 2018
1 parent d135525 commit 789b8ab
Show file tree
Hide file tree
Showing 7 changed files with 47 additions and 4 deletions.
6 changes: 4 additions & 2 deletions app/controllers/courses_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,10 @@ def create
payload = {class: @course.to_roster_hash}
response = request_roster_api('/classes/', :post, payload)
@course.update_attributes!(sourced_id: response['class']['sourcedId'])
# FIXME: update course manager
@course.managers.each do |manager|
payload = {enrollment: CourseMember.to_roster_hash(@course.sourced_id, manager, 'manager')}
request_roster_api('/enrollments/', :post, payload)
end
end
end
set_nav_session 'repository', 'courses', @course.id
Expand Down Expand Up @@ -62,7 +65,6 @@ def update
if SYSTEM_ROSTER_SYNC == :on
payload = {class: @course.to_roster_hash}
response = request_roster_api("/classes/#{@course.sourced_id}", :put, payload)
# FIXME: update course manager
end
end
record_user_action('updated', @course.id)
Expand Down
23 changes: 23 additions & 0 deletions app/models/course_member.rb
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,29 @@ def self.update_managers(course_id, current_ids, ids)
return false
end

def self.to_roster_hash course_sourced_id, user, role
raise if user.sourced_id.nil?
hash = {
classSourcedId: course_sourced_id,
schoolSourcedId: Rails.application.secrets.roster_school_sourced_id,
userSourcedId: user.sourced_id,
role: self.to_roster_role(role)
}
end

def self.to_roster_role role
case role
when 'manager'
'teacher'
when 'assistant'
'aide'
when 'learner'
'student'
else
raise
end
end

def deletable?
stickies = Sticky.where(course_id: course_id, manager_id: user_id)
case role
Expand Down
5 changes: 4 additions & 1 deletion app/models/user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
# last_signin_at :datetime
# created_at :datetime not null
# updated_at :datetime not null
# sourced_id :string
#

require 'net/ldap'
Expand Down Expand Up @@ -53,6 +54,7 @@ class User < ApplicationRecord
validates_presence_of :signin_name
validates_presence_of :token
validates_uniqueness_of :signin_name
validates_uniqueness_of :sourced_id, allow_nil: true
validates_uniqueness_of :token
validates_inclusion_of :authentication, in: %w[local ldap]
validates_inclusion_of :role, in: %w[admin manager user suspended]
Expand Down Expand Up @@ -105,8 +107,9 @@ def self.sync_roster(rusers)

ids = []
rusers.each do |ru|
# Query by signin_name in consideration of the case there are users without sourcedId created before using RosterAPI
user = User.find_or_initialize_by(signin_name: ru['username'])
if user.update_attributes(authentication: 'ldap', family_name: ru['familyName'], given_name: ru['givenName'])
if user.update_attributes(sourced_id: ru['sourcedId'], authentication: 'ldap', family_name: ru['familyName'], given_name: ru['givenName'])
ids.push user.id
end
end
Expand Down
2 changes: 1 addition & 1 deletion config/initializers/constants.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@

# Synchronization mode with IMS OneRoster API: select one from [:on, :suspended, :off]
# Check schedule.rb and run "bundle exec whenever -i" when SYSTEM_ROSTER_SYNC changes
SYSTEM_ROSTER_SYNC = :off.freeze
SYSTEM_ROSTER_SYNC = :on.freeze

# ===== Versatile constatns =====
# time delay for autocomplete
Expand Down
6 changes: 6 additions & 0 deletions db/migrate/20181126082338_add_sourced_id_to_users.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
class AddSourcedIdToUsers < ActiveRecord::Migration[5.0]
def change
add_column :users, :sourced_id, :string, after: :id
add_index :users, :sourced_id, unique: true
end
end
2 changes: 2 additions & 0 deletions test/factories/users.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,12 @@
# last_signin_at :datetime
# created_at :datetime not null
# updated_at :datetime not null
# sourced_id :string
#

FactoryBot.define do
factory :user, class: User do
sequence(:sourced_id) { |i| "dummy-user-sourced_id-#{i}" }
sequence(:signin_name) { |i| "user#{i}-test" }
password 'temporary'
password_confirmation 'temporary'
Expand Down
7 changes: 7 additions & 0 deletions test/models/user_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
# last_signin_at :datetime
# created_at :datetime not null
# updated_at :datetime not null
# sourced_id :string
#

require 'test_helper'
Expand Down Expand Up @@ -70,6 +71,12 @@ class UserTest < ActiveSupport::TestCase
assert_invalid build(:user, signin_name: user.signin_name), :signin_name
end

# test for validates_uniqueness_of :sourced_id, allow_nil: true
test 'some users with same sourced_id are invalid' do
user = create(:user)
assert_invalid build(:user, sourced_id: user.sourced_id), :sourced_id
end

# test for validates_uniqueness_of :token
test 'some users with same token are invalid' do
user = create(:user)
Expand Down

0 comments on commit 789b8ab

Please sign in to comment.