From 789b8ab044a4f43560f57b432bed68183bc7779c Mon Sep 17 00:00:00 2001 From: Koichi Yoshizaki Date: Mon, 26 Nov 2018 23:26:30 +0900 Subject: [PATCH] :wrench: Sync Course managers to API server when creating course, #151 - Request to original API with HTTP POST/PUT/DELETE method for Course resource --- app/controllers/courses_controller.rb | 6 +++-- app/models/course_member.rb | 23 +++++++++++++++++++ app/models/user.rb | 5 +++- config/initializers/constants.rb | 2 +- .../20181126082338_add_sourced_id_to_users.rb | 6 +++++ test/factories/users.rb | 2 ++ test/models/user_test.rb | 7 ++++++ 7 files changed, 47 insertions(+), 4 deletions(-) create mode 100644 db/migrate/20181126082338_add_sourced_id_to_users.rb diff --git a/app/controllers/courses_controller.rb b/app/controllers/courses_controller.rb index 2b2d83b..1614035 100644 --- a/app/controllers/courses_controller.rb +++ b/app/controllers/courses_controller.rb @@ -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 @@ -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) diff --git a/app/models/course_member.rb b/app/models/course_member.rb index e11ef63..90954fd 100644 --- a/app/models/course_member.rb +++ b/app/models/course_member.rb @@ -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 diff --git a/app/models/user.rb b/app/models/user.rb index c1955f0..4a136ed 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -20,6 +20,7 @@ # last_signin_at :datetime # created_at :datetime not null # updated_at :datetime not null +# sourced_id :string # require 'net/ldap' @@ -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] @@ -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 diff --git a/config/initializers/constants.rb b/config/initializers/constants.rb index e4ea3ac..6a6fbea 100644 --- a/config/initializers/constants.rb +++ b/config/initializers/constants.rb @@ -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 diff --git a/db/migrate/20181126082338_add_sourced_id_to_users.rb b/db/migrate/20181126082338_add_sourced_id_to_users.rb new file mode 100644 index 0000000..29f016e --- /dev/null +++ b/db/migrate/20181126082338_add_sourced_id_to_users.rb @@ -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 diff --git a/test/factories/users.rb b/test/factories/users.rb index a73a68b..63735c2 100644 --- a/test/factories/users.rb +++ b/test/factories/users.rb @@ -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' diff --git a/test/models/user_test.rb b/test/models/user_test.rb index a84dc95..c117fcb 100644 --- a/test/models/user_test.rb +++ b/test/models/user_test.rb @@ -20,6 +20,7 @@ # last_signin_at :datetime # created_at :datetime not null # updated_at :datetime not null +# sourced_id :string # require 'test_helper' @@ -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)