diff --git a/app/models/room.rb b/app/models/room.rb index 1736f80c64..5620e78113 100644 --- a/app/models/room.rb +++ b/app/models/room.rb @@ -30,7 +30,7 @@ class Room < ApplicationRecord validates :name, presence: true validates :friendly_id, presence: true, uniqueness: true validates :meeting_id, presence: true, uniqueness: true - validates :voice_bridge, uniqueness: true + # validates :voice_bridge, uniqueness: true validates :presentation, content_type: Rails.configuration.uploads[:presentations][:formats], size: { less_than: Rails.configuration.uploads[:presentations][:max_size] } @@ -38,7 +38,7 @@ class Room < ApplicationRecord validates :name, length: { minimum: 1, maximum: 255 } validates :recordings_processing, numericality: { only_integer: true, greater_than_or_equal_to: 0 } - before_validation :set_friendly_id, :set_meeting_id, :set_voice_brige, on: :create + before_validation :set_friendly_id, :set_meeting_id, :set_voice_bridge, on: :create before_save :scan_presentation_for_virus after_create :create_meeting_options @@ -103,18 +103,22 @@ def set_meeting_id retry end - # Create unique pin for voice brige max 10^x - x*0.1 unique ids (x = len of pin) + # Create unique pin for voice brige max 10^x - 10^(x-1) unique ids (x = len of pin) # No leading Zeros - def set_voice_brige + def set_voice_bridge + self.voice_bridge = nil + return if Rails.application.config.voice_bridge_phone_number.nil? pin_len = Rails.application.config.sip_pin_length max_pins = 10.pow(pin_len) - 10.pow(pin_len - 1) - 1 - return if Room.all.where.not(voice_bridge: nil).length > max_pins # Check if a pins are left + return if Room.all.where.not(voice_bridge: nil).length >= max_pins # Check if pins are left - id = SecureRandom.random_number(10.pow(pin_len) - 1) # Pick random pin - id = ((id + 1) % max_pin) + 10.pow(pin_len - 1) while Room.exists?(voice_bridge: id) || id < 10.pow(pin_len - 1) # Take next free pin + # Pick random pin + id = SecureRandom.random_number(10.pow(pin_len) - 1) + # Ensure uniqueness and if not take next free pin + id = ((id + 1) % max_pins) + 10.pow(pin_len - 1) while Room.exists?(voice_bridge: id) || id < 10.pow(pin_len - 1) self.voice_bridge = id end diff --git a/db/migrate/20230321125010_add_voice_brige_to_romms.rb b/db/migrate/20230321125010_add_voice_brige_to_romms.rb index 2b87a390d9..829e427e07 100644 --- a/db/migrate/20230321125010_add_voice_brige_to_romms.rb +++ b/db/migrate/20230321125010_add_voice_brige_to_romms.rb @@ -3,6 +3,6 @@ class AddVoiceBrigeToRomms < ActiveRecord::Migration[7.0] def change add_column :rooms, :voice_bridge, :integer, null: true, default: nil - add_index :rooms, :voice_bridge, unique: true + add_index :rooms, :voice_bridge end end diff --git a/spec/factories/room_factory.rb b/spec/factories/room_factory.rb index e94153021a..7258496f58 100644 --- a/spec/factories/room_factory.rb +++ b/spec/factories/room_factory.rb @@ -21,6 +21,6 @@ user name { Faker::Educator.course_name } last_session { nil } - # meeting_id & friendly_id are set automatically using before_validation + # meeting_id & friendly_id & voice_brige are set automatically using before_validation end end diff --git a/spec/models/room_spec.rb b/spec/models/room_spec.rb index 691d47f7e4..c5b7292c3a 100644 --- a/spec/models/room_spec.rb +++ b/spec/models/room_spec.rb @@ -30,7 +30,7 @@ it { is_expected.to validate_length_of(:name).is_at_least(1).is_at_most(255) } it { is_expected.to validate_numericality_of(:recordings_processing).only_integer.is_greater_than_or_equal_to(0) } - # Can't test validation on friendly_id and meeting_id due to before_validations + # Can't test validation on friendly_id, meeting_id and voice_brige due to before_validations context 'presentation validations' do it 'fails if the presentation is not a valid extension' do @@ -84,6 +84,23 @@ expect { duplicate_room.meeting_id = room.meeting_id }.to change { duplicate_room.valid? }.to false end end + + describe '#set_voice_brige' do + it 'sets a rooms voice_brige before creating' do + if Rails.application.config.voice_bridge_phone_number.nil? + expect(room.voice_bridge).to be_nil + else + expect(room.voice_bridge).to be_present + end + end + + it 'prevents duplicate voice_briges' do + duplicate_room = create(:room) + unless Rails.application.config.voice_bridge_phone_number.nil? + expect { duplicate_room.voice_bridge = room.voice_bridge }.to change { duplicate_room.valid? }.to false + end + end + end end describe 'before_save' do