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

LTI-388, LTI-389, LTI-390: Improvements and fixes to parameter passing #331

Closed
wants to merge 3 commits into from
Closed
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: 2 additions & 4 deletions app/controllers/concerns/bbb_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -288,11 +288,9 @@ def string_to_bool(value)
# - action: either 'join' or 'create'
# - options: the hash of params sent as part of the request
def add_ext_params(action, options)
ext_params = tenant_setting(@chosen_room.tenant, 'ext_params')

@chosen_room.settings['ext_params']&.[](action)&.each do |key, value|
@extra_params_to_bbb[action]&.each do |key, value|
# the value in ext_params from the tenant settings is the name that should be passed to BBB
bbb_name = ext_params&.[](action)&.[](key)
bbb_name = @broker_ext_params&.[](action)&.[](key)
options[bbb_name] = value if bbb_name
end
end
Expand Down
68 changes: 57 additions & 11 deletions app/controllers/rooms_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ class RoomsController < ApplicationController
before_action :allow_iframe_requests
before_action :set_current_locale
before_action :set_action_cable, only: %i[launch]
before_action :set_ext_params, only: [:show]
before_action :set_ext_params, except: [:launch]

after_action :broadcast_meeting, only: [:meeting_end]

Expand Down Expand Up @@ -524,20 +524,66 @@ def set_action_cable
end

def set_ext_params
logger.debug('[Rooms\'s Controller] Setting ext_params in room controller.')
tenant = @room.tenant
broker_ext_params = tenant_setting(tenant, 'ext_params')
lms_custom_params = launch_request_params['message']['custom_params']
logger.debug('[Rooms Controller] Setting ext_params in room controller.')
tenant = @chosen_room.tenant
@broker_ext_params ||= tenant_setting(tenant, 'ext_params')

logger.debug("[Rooms\'s Controller] extra params from broker: #{broker_ext_params} \n custom params from lms: #{lms_custom_params}")
launch_params = if Rails.configuration.cache_enabled
Rails.cache.fetch("rooms/#{@chosen_room.handler}/tenant/#{tenant}/user/#{@user.uid}/launch_params",
expires_in: Rails.configuration.cache_expires_in_minutes.minutes) do
logger.debug('fetching launch params for extra params from cache')
launch_request_params['message']
end
else
launch_request_params['message']
end

pass_on_join_params = lms_custom_params.select { |k, _| broker_ext_params&.[]('join')&.key?(k) }
pass_on_create_params = lms_custom_params.select { |k, _| broker_ext_params&.[]('create')&.key?(k) }
logger.debug("[Rooms Controller] extra params from broker for room #{@chosen_room.name}: #{@broker_ext_params}")

@room.add_settings({ ext_params: { 'join' => pass_on_join_params, 'create' => pass_on_create_params } })
pass_on_join_params = launch_and_extra_params_intersection_hash(launch_params, 'join', @broker_ext_params&.[]('join'))
pass_on_create_params = launch_and_extra_params_intersection_hash(launch_params, 'create', @broker_ext_params&.[]('create'))

logger.debug("[Rooms\'s Controller] Set the following external params for room #{@room.id}: #{@room.settings['ext_params'].to_json}")
@extra_params_to_bbb = { 'join' => pass_on_join_params, 'create' => pass_on_create_params }

logger.debug("[Rooms Controller] The extra parameters to be passed to BBB are: #{@extra_params_to_bbb}")
rescue StandardError => e
logger.error("[Rooms\'s Controller] Error setting extra parameters: #{e}")
logger.error("[Rooms Controller] Error setting extra parameters: #{e}")
end

# return a hash of key:value pairs from the launch_params,
# for keys that exist in the extra params hash retrieved from the broker settings
def launch_and_extra_params_intersection_hash(launch_params, action, actions_hash)
if Rails.configuration.cache_enabled
Rails.cache.fetch("rooms/#{@chosen_room.handler}/tenant/#{@chosen_room.tenant}/user/#{@user.uid}/ext_#{action}_params",
expires_in: Rails.configuration.cache_expires_in_minutes.minutes) do
calculate_intersection_hash(launch_params, actions_hash)
end
else
calculate_intersection_hash(launch_params, actions_hash)
end
end

def calculate_intersection_hash(launch_params, actions_hash)
result = {}
actions_hash&.each_key do |key|
value = find_launch_param(launch_params, key)
result[key] = value if value
end
result
end

# Check if the launch params contain a certain param
# If they do, return the value of that param
def find_launch_param(launch_params, key)
return launch_params[key] if launch_params.key?(key)

launch_params.each_value do |value|
if value.is_a?(Hash)
result = find_launch_param(value, key)
return result unless result.nil?
end
end

nil
end
end
8 changes: 0 additions & 8 deletions app/models/room.rb
Original file line number Diff line number Diff line change
Expand Up @@ -53,14 +53,6 @@ def self.recording_setting?(setting)
RECORDING_SETTINGS.include?(setting.to_sym)
end

# Add key-value paris to the settings jsonb column
# new-settings is a hash
def add_settings(new_settings)
updated_settings = settings.deep_merge(new_settings)

update(settings: updated_settings)
end

private

def random_password(length, reference = '')
Expand Down
11 changes: 9 additions & 2 deletions spec/concerns/bbb_helper_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,13 @@

before do
@room = @chosen_room = create(:room)
@extra_params_to_bbb = { join: { 'custom_one' => 'this is one' }, create: { 'custom_two' => 'this is two' } }
@broker_ext_params = {
'join' =>
{ 'custom_one' => 'userdata-bbb_one' },
'create' =>
{ 'custom_two' => 'meta_bbb_two' },
}
allow_any_instance_of(BbbHelper).to(receive(:bbb).and_return(bbb_api))
allow_any_instance_of(BrokerHelper).to(receive(:broker_tenant_info).and_return({
'handler_params' => 'context_id',
Expand All @@ -38,9 +45,9 @@
'bigbluebutton_moderator_roles' => 'administrator,teacher',
'ext_params' => {
'join' =>
{ 'custom_user_image' => 'ext_user_image' },
{ 'custom_one' => 'userdata-bbb_one' },
'create' =>
{ 'custom_context_id' => 'ext_course_id' },
{ 'custom_two' => 'meta_bbb_two' },
},
}))
end
Expand Down
17 changes: 17 additions & 0 deletions spec/controllers/rooms_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,17 @@
before :each do
allow_any_instance_of(RoomsController).to(receive(:authenticate_user!).and_return(:success))
allow_any_instance_of(RoomsController).to(receive(:bbb).and_return(bbb_api))
allow_any_instance_of(RoomsController).to(receive(:launch_request_params)).and_return({
'token' => '319bda5a0141d39e003767cf3b4f675b',
'valid' => true,
'tenant' => 'test',
'message' => {
'custom_params' => {
'custom_one' => 'this is one',
'custom_two' => 'this is two',
},
},
})
allow_any_instance_of(NotifyMeetingWatcherJob).to(receive(:bbb).and_return(bbb_api)) # stub actioncable processes
allow_any_instance_of(BrokerHelper).to(receive(:broker_tenant_info).and_return({
'handler_params' => 'context_id',
Expand All @@ -17,6 +28,12 @@
'bigbluebutton_secret' => 'supersecretsecret',
'enable_shared_rooms' => 'true',
'bigbluebutton_moderator_roles' => 'administrator,teacher',
'ext_params' => {
'join' =>
{ 'custom_one' => 'userdata-one' },
'create' =>
{ 'custom_two' => 'meta_two' },
},
}))

@request.session['handler'] = {
Expand Down