From 6ea073747ee13a2c795110b0339eda7a3c8319c1 Mon Sep 17 00:00:00 2001
From: Khemissi Amir <amir.khemissi@insat.ucar.tn>
Date: Tue, 4 Apr 2023 17:04:37 +0100
Subject: [PATCH] Fixes to complete the relative URL root path support.
(#5106)
* Action cable: Added support for relative URL root path.
* Meeting start: Added support for relative URL root path.
* Application: Minor improvements to relative URL root path parsing.
* Email branding: Added support for relative URL root path.
+ Refactored and improved Setting getter service.
* Manage users: Fixed edit user on custom relative URL root path.
---
app/controllers/api/v1/meetings_controller.rb | 4 +-
.../api/v1/site_settings_controller.rb | 10 +---
app/javascript/channels/consumer.jsx | 2 +-
.../hooks/queries/users/useUser.jsx | 4 +-
app/mailers/user_mailer.rb | 3 +-
app/services/meeting_starter.rb | 2 +-
app/services/setting_getter.rb | 53 +++++++++++--------
config/application.rb | 4 +-
esbuild.dev.mjs | 3 +-
esbuild.mjs | 3 +-
spec/controllers/meetings_controller_spec.rb | 4 +-
.../site_settings_controller_spec.rb | 10 ++--
12 files changed, 53 insertions(+), 49 deletions(-)
diff --git a/app/controllers/api/v1/meetings_controller.rb b/app/controllers/api/v1/meetings_controller.rb
index ef823e07..3b5126fa 100644
--- a/app/controllers/api/v1/meetings_controller.rb
+++ b/app/controllers/api/v1/meetings_controller.rb
@@ -29,7 +29,7 @@ module Api
# Starts a BigBlueButton meetings and joins in the meeting starter
def start
begin
- MeetingStarter.new(room: @room, base_url: root_url, current_user:, provider: current_provider).call
+ MeetingStarter.new(room: @room, base_url: request.base_url, current_user:, provider: current_provider).call
rescue BigBlueButton::BigBlueButtonException => e
return render_error status: :bad_request unless e.key == 'idNotUnique'
end
@@ -68,7 +68,7 @@ module Api
if !data[:status] && settings['glAnyoneCanStart'] == 'true' # Meeting isnt running and anyoneCanStart setting is enabled
begin
- MeetingStarter.new(room: @room, base_url: root_url, current_user:, provider: current_provider).call
+ MeetingStarter.new(room: @room, base_url: request.base_url, current_user:, provider: current_provider).call
rescue BigBlueButton::BigBlueButtonException => e
return render_error status: :bad_request unless e.key == 'idNotUnique'
end
diff --git a/app/controllers/api/v1/site_settings_controller.rb b/app/controllers/api/v1/site_settings_controller.rb
index 8bf06d0d..ecbd9472 100644
--- a/app/controllers/api/v1/site_settings_controller.rb
+++ b/app/controllers/api/v1/site_settings_controller.rb
@@ -25,17 +25,9 @@ module Api
# GET /api/v1/site_settings
# Returns the values of 1 or multiple site_settings that are not forbidden to access
def index
- settings = {}
return render_error status: :forbidden if forbidden_settings(params[:names])
- if params[:names].is_a?(Array)
- params[:names].each do |name|
- settings[name] = SettingGetter.new(setting_name: name, provider: current_provider).call
- end
- else
- # return the value directly
- settings = SettingGetter.new(setting_name: params[:names], provider: current_provider).call
- end
+ settings = SettingGetter.new(setting_name: params[:names], provider: current_provider).call
render_data data: settings, status: :ok
end
diff --git a/app/javascript/channels/consumer.jsx b/app/javascript/channels/consumer.jsx
index 99123c20..e1d523c5 100644
--- a/app/javascript/channels/consumer.jsx
+++ b/app/javascript/channels/consumer.jsx
@@ -19,4 +19,4 @@
import { createConsumer } from '@rails/actioncable';
-export default createConsumer();
+export default createConsumer(`${process.env.RELATIVE_URL_ROOT}/cable`);
diff --git a/app/javascript/hooks/queries/users/useUser.jsx b/app/javascript/hooks/queries/users/useUser.jsx
index 64f22960..54c460bb 100644
--- a/app/javascript/hooks/queries/users/useUser.jsx
+++ b/app/javascript/hooks/queries/users/useUser.jsx
@@ -15,11 +15,11 @@
// with Greenlight; if not, see <http://www.gnu.org/licenses/>.
import { useQuery } from 'react-query';
-import axios from 'axios';
+import axios from '../../../helpers/Axios';
export default function useUser(userId) {
return useQuery(
['getUser', userId],
- () => axios.get(`/api/v1/users/${userId}.json`).then((resp) => resp.data.data),
+ () => axios.get(`/users/${userId}.json`).then((resp) => resp.data.data),
);
}
diff --git a/app/mailers/user_mailer.rb b/app/mailers/user_mailer.rb
index 2df77808..3e00b6fc 100644
--- a/app/mailers/user_mailer.rb
+++ b/app/mailers/user_mailer.rb
@@ -55,8 +55,7 @@ class UserMailer < ApplicationMailer
end
def branding
- branding_hash = SiteSetting.includes(:setting).where(provider: @provider, settings: { name: %w[PrimaryColor BrandingImage] })
- .pluck(:name, :value).to_h
+ branding_hash = SettingGetter.new(setting_name: %w[PrimaryColor BrandingImage], provider: @provider).call
@brand_image = ActionController::Base.helpers.image_url(branding_hash['BrandingImage'], host: @base_url)
@brand_color = branding_hash['PrimaryColor']
end
diff --git a/app/services/meeting_starter.rb b/app/services/meeting_starter.rb
index e576082c..4575aef4 100644
--- a/app/services/meeting_starter.rb
+++ b/app/services/meeting_starter.rb
@@ -56,7 +56,7 @@ class MeetingStarter
private
def computed_options(access_code:)
- room_url = File.join(@base_url, '/rooms/', @room.friendly_id, '/join')
+ room_url = "#{root_url(host: @base_url)}rooms/#{@room.friendly_id}/join"
moderator_message = "#{I18n.t('meeting.moderator_message')}<br>#{room_url}"
moderator_message += "<br>#{I18n.t('meeting.access_code', code: access_code)}" if access_code.present?
{
diff --git a/app/services/setting_getter.rb b/app/services/setting_getter.rb
index 4c5f4352..fda7f838 100644
--- a/app/services/setting_getter.rb
+++ b/app/services/setting_getter.rb
@@ -20,40 +20,51 @@ class SettingGetter
include Rails.application.routes.url_helpers
def initialize(setting_name:, provider:)
- @setting_name = setting_name
+ @setting_name = Array(setting_name)
@provider = provider
end
def call
- setting = SiteSetting.joins(:setting)
- .find_by(
- provider: @provider,
- setting: { name: @setting_name }
- )
-
- value = if @setting_name == 'BrandingImage'
- if setting.image.attached?
- rails_blob_path setting.image, only_path: true
- else
- ActionController::Base.helpers.image_path('bbb_logo.png')
- end
- else
- setting&.value
- end
-
- transform_value(value)
+ # Fetch the site settings records while eager loading their respective settings ↓
+ site_settings = SiteSetting.includes(:setting)
+ .where(
+ provider: @provider,
+ setting: { name: @setting_name }
+ )
+
+ # Pessimist check: Pass only if all provided names were found ↓
+ return nil unless @setting_name.size == site_settings.size
+
+ site_settings_hash = {}
+
+ # In memory prepare the result hash ↓
+ site_settings.map do |site_setting|
+ site_settings_hash[site_setting.setting.name] = transform_value(site_setting)
+ end
+
+ # If there's only one setting is being fetched no need for a hash ↓
+ return site_settings_hash.values.first if site_settings_hash.size == 1
+
+ # A Hash<setting_name => parsed_value> is returned otherwise ↓
+ site_settings_hash
end
private
- def transform_value(value)
- case value
+ def transform_value(site_setting)
+ if site_setting.setting.name == 'BrandingImage'
+ return rails_blob_path site_setting.image, only_path: true if site_setting.image.attached?
+
+ return ActionController::Base.helpers.image_path('bbb_logo.png')
+ end
+
+ case site_setting.value
when 'true'
true
when 'false'
false
else
- value
+ site_setting.value
end
end
end
diff --git a/config/application.rb b/config/application.rb
index 3c50cf78..71810992 100644
--- a/config/application.rb
+++ b/config/application.rb
@@ -66,6 +66,8 @@ module Greenlight
config.bigbluebutton_secret = ENV.fetch('BIGBLUEBUTTON_SECRET', '8cd8ef52e8e101574e400365b55e11a6')
- config.relative_url_root = ENV.fetch('RELATIVE_URL_ROOT', '/')
+ # Fetch 'RELATIVE_URL_ROOT' ENV variable value while removing any trailing slashes.
+ config.relative_url_root = ENV.fetch('RELATIVE_URL_ROOT', nil)&.sub(%r{/*\z}, '')
+ config.relative_url_root = '/' if config.relative_url_root.blank?
end
end
diff --git a/esbuild.dev.mjs b/esbuild.dev.mjs
index fb48838b..83a11e9c 100644
--- a/esbuild.dev.mjs
+++ b/esbuild.dev.mjs
@@ -1,6 +1,7 @@
import * as esbuild from 'esbuild';
-const relativeUrlRoot = (process.env.RELATIVE_URL_ROOT || '').replace(/\/$/, '');
+// Fetch 'RELATIVE_URL_ROOT' ENV variable value while removing any trailing slashes.
+const relativeUrlRoot = (process.env.RELATIVE_URL_ROOT || '').replace(/\/*$/, '');
await esbuild.build({
entryPoints: ['app/javascript/main.jsx'],
diff --git a/esbuild.mjs b/esbuild.mjs
index 7b76ffbf..8a658959 100644
--- a/esbuild.mjs
+++ b/esbuild.mjs
@@ -1,6 +1,7 @@
import * as esbuild from 'esbuild';
-const relativeUrlRoot = (process.env.RELATIVE_URL_ROOT || '').replace(/\/$/, '');
+// Fetch 'RELATIVE_URL_ROOT' ENV variable value while removing any trailing slashes.
+const relativeUrlRoot = (process.env.RELATIVE_URL_ROOT || '').replace(/\/*$/, '');
await esbuild.build({
entryPoints: ['app/javascript/main.jsx'],
diff --git a/spec/controllers/meetings_controller_spec.rb b/spec/controllers/meetings_controller_spec.rb
index 66827d38..21230bb1 100644
--- a/spec/controllers/meetings_controller_spec.rb
+++ b/spec/controllers/meetings_controller_spec.rb
@@ -41,7 +41,7 @@ RSpec.describe Api::V1::MeetingsController, type: :controller do
describe '#start' do
it 'makes a call to the MeetingStarter service with the right values and returns the join url' do
- expect(MeetingStarter).to receive(:new).with(room:, base_url: root_url, current_user: user, provider: 'greenlight').and_call_original
+ expect(MeetingStarter).to receive(:new).with(room:, base_url: request.base_url, current_user: user, provider: 'greenlight').and_call_original
expect_any_instance_of(MeetingStarter).to receive(:call)
expect_any_instance_of(BigBlueButtonApi).to receive(:join_meeting).with(room:, name: user.name, avatar_url: nil, role: 'Moderator')
@@ -343,7 +343,7 @@ RSpec.describe Api::V1::MeetingsController, type: :controller do
allow_any_instance_of(BigBlueButtonApi).to receive(:meeting_running?).and_return(false)
- expect(MeetingStarter).to receive(:new).with(room:, base_url: root_url, current_user: user, provider: 'greenlight').and_call_original
+ expect(MeetingStarter).to receive(:new).with(room:, base_url: request.base_url, current_user: user, provider: 'greenlight').and_call_original
expect_any_instance_of(MeetingStarter).to receive(:call)
post :status, params: { friendly_id: room.friendly_id, name: user.name }
diff --git a/spec/controllers/site_settings_controller_spec.rb b/spec/controllers/site_settings_controller_spec.rb
index f049573d..8b2faf14 100644
--- a/spec/controllers/site_settings_controller_spec.rb
+++ b/spec/controllers/site_settings_controller_spec.rb
@@ -38,15 +38,13 @@ RSpec.describe Api::V1::SiteSettingsController, type: :controller do
end
it 'calls SettingGetter and returns multiple values' do
- expect(SettingGetter).to receive(:new).with(setting_name: 'SettingName', provider: 'greenlight').and_call_original
- expect(SettingGetter).to receive(:new).with(setting_name: 'SettingName2', provider: 'greenlight').and_call_original
- allow_any_instance_of(SettingGetter).to receive(:call).and_return('false')
+ expect(SettingGetter).to receive(:new).with(setting_name: %w[Uno Dos Tres], provider: 'greenlight').and_call_original
+ allow_any_instance_of(SettingGetter).to receive(:call).and_return({ 'Uno' => 1, 'Dos' => 2, 'Tres' => 3 })
- get :index, params: { names: %w[SettingName SettingName2] }
+ get :index, params: { names: %w[Uno Dos Tres] }
expect(response).to have_http_status(:ok)
- expect(JSON.parse(response.body)['data']['SettingName']).to eq('false')
- expect(JSON.parse(response.body)['data']['SettingName2']).to eq('false')
+ expect(JSON.parse(response.body)['data']).to eq({ 'Uno' => 1, 'Dos' => 2, 'Tres' => 3 })
end
it 'returns forbidden if trying to access a forbidden setting' do
--
GitLab