-
Notifications
You must be signed in to change notification settings - Fork 27
Static dialin pin #2116
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
base: develop
Are you sure you want to change the base?
Static dialin pin #2116
Conversation
## Walkthrough
This change introduces a "dial-in PIN" feature for rooms, including backend support, validation, database schema updates, API resource adjustments, and frontend UI components. It adds new settings for room types and rooms, validation logic, translations, and UI elements for managing and displaying dial-in PINs, including enforcement and default behaviors.
## Changes
| Files / Group | Change Summary |
|--------------------------------------------------------------------------------------------------------------|---------------|
| `database/migrations/2025_05_05_222245_add_dialin_pin_to_rooms_table.php` | Adds `dialin_pin` column to `rooms` table and `has_dialin_pin_enforced`, `has_dialin_pin_default` columns to `room_types` table. |
| `app/Models/Room.php`<br>`app/Models/RoomType.php` | Adds attribute casting for `dialin_pin` (as integer) in Room and for `has_dialin_pin_default`/`has_dialin_pin_enforced` (as boolean) in RoomType. |
| `app/Http/Controllers/api/v1/RoomController.php` | Generates and updates `dialin_pin` in store and update methods based on room type settings. |
| `app/Http/Controllers/api/v1/RoomTypeController.php` | Handles `has_dialin_pin_default` and `has_dialin_pin_enforced` in store and update methods. |
| `app/Http/Requests/RoomTypeRequest.php` | Adds validation rules and attribute names for `has_dialin_pin_default` and `has_dialin_pin_enforced`. |
| `app/Http/Requests/UpdateRoomSettings.php` | Adds validation for `dialin_pin` with dynamic rules based on room type; new private method for rule generation. |
| `app/Http/Resources/RoomSettings.php` | Adds `dialin_pin` to the resource output array. |
| `app/Http/Resources/RoomType.php` | Adds `has_dialin_pin_default` and `has_dialin_pin_enforced` to default room settings output. |
| `app/Services/MeetingService.php` | Sets `voiceBridge` parameter using room's `dialin_pin` when starting a meeting. |
| `resources/js/components/RoomTabSettings.vue` | Adds dial-in PIN input to advanced room settings form. |
| `resources/js/components/RoomTabSettingsDialinPinInput.vue` | New component for dial-in PIN input, generation, enforcement/prohibition display, and error handling. |
| `resources/js/composables/useRoomTypeSettings.js` | Adds dial-in PIN switch to advanced room type settings. |
| `resources/js/constants/roomSettings.js` | Adds `dialin_pin` as an expert room setting. |
| `resources/js/views/AdminRoomTypesView.vue` | Adds UI controls for `has_dialin_pin_default` and `has_dialin_pin_enforced` in room type admin view. |
| `lang/en/rooms.php`<br>`lang/de/rooms.php` | Adds localization entries for dial-in PIN feature and related UI messages. |
| `lang/en/validation.php`<br>`lang/de/validation.php` | Adds attribute translations for `dialin_pin` and `has_dialin_pin`. |
## Sequence Diagram(s)
```mermaid
sequenceDiagram
participant Admin as Admin User
participant AdminUI as AdminRoomTypesView.vue
participant API as RoomTypeController
participant DB as Database
Admin->>AdminUI: Toggle "Has Dialin PIN" / "Enforced" switches
AdminUI->>API: POST/PUT room type with dialin PIN settings
API->>DB: Update room_types (has_dialin_pin_default, has_dialin_pin_enforced)
API-->>AdminUI: Confirmation/updated room type
participant User as User
participant RoomUI as RoomTabSettings.vue
participant RoomAPI as RoomController
participant RoomDB as Database
User->>RoomUI: Set or generate dial-in PIN
RoomUI->>RoomAPI: POST/PUT room with dialin_pin
RoomAPI->>RoomDB: Store/Update room (dialin_pin)
RoomAPI-->>RoomUI: Confirmation/updated room
participant MeetingService as MeetingService
participant BBB as BigBlueButton
User->>MeetingService: Start meeting for room
MeetingService->>BBB: Create meeting (voiceBridge = dialin_pin)
BBB-->>MeetingService: Meeting startedSuggested labels
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (3)
lang/de/rooms.php (1)
362-367: Minor inconsistency in German translationsThere's a small inconsistency in how "Einwahl-PIN" (line 5) vs "Einwahl Pin" (lines 362-367) is written - with a hyphen in one place but not in others.
Consider standardizing the terminology for consistency:
- 'has_dialin_pin' => 'Einwahl PIN', - 'dialin_pin_none_placeholder' => '-- Zufällig --', - 'dialin_pin_enforced' => 'Die Raumart erzwingt, dass ein Einwahl Pin existiert', - 'dialin_pin_prohibited' => 'Die Raumart erzwingt, dass kein Einwahl Pin existiert', - 'generate_dialin_pin' => 'Neuen Einwahl Pin erstellen', - 'delete_dialin_pin' => 'Einwahl Pin entfernen', + 'has_dialin_pin' => 'Einwahl-PIN', + 'dialin_pin_none_placeholder' => '-- Zufällig --', + 'dialin_pin_enforced' => 'Die Raumart erzwingt, dass ein Einwahl-PIN existiert', + 'dialin_pin_prohibited' => 'Die Raumart erzwingt, dass kein Einwahl-PIN existiert', + 'generate_dialin_pin' => 'Neuen Einwahl-PIN erstellen', + 'delete_dialin_pin' => 'Einwahl-PIN entfernen',resources/js/components/RoomTabSettingsDialinPinInput.vue (2)
109-112: Small issue in random PIN generation logicThere's a minor issue in the random number generation formula that might lead to slightly uneven distribution.
- model.value[props.setting] = - Math.floor(Math.random() * (99999 - 11112)) + 11111; + model.value[props.setting] = + Math.floor(Math.random() * (99999 - 11111 + 1)) + 11111;The current formula subtracts 11112 from 99999, which results in 88887 possible values rather than 88889 (99999-11111+1). This would make the number 99999 slightly less likely to be generated.
1-113: Consider adding a validation method for dial-in PINWhile the backend validation will likely handle invalid PINs, it could be beneficial to add frontend validation to provide immediate feedback to users.
Consider adding a method to validate the PIN format (e.g., ensure it's a 5-digit number) before submitting to the server. This could be implemented as follows:
/** * Validate the dialin PIN format * @returns {boolean} True if valid, false otherwise */ function validateDialinPin() { if (!model.value[props.setting]) return true; // Empty is valid (handled by backend if required) const pin = model.value[props.setting]; return Number.isInteger(pin) && pin >= 10000 && pin <= 99999; } // You could then watch for changes and validate watch(() => model.value[props.setting], (newVal) => { if (newVal && !validateDialinPin()) { // Set a local validation error state or emit an event } });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (19)
app/Http/Controllers/api/v1/RoomController.php(2 hunks)app/Http/Controllers/api/v1/RoomTypeController.php(2 hunks)app/Http/Requests/RoomTypeRequest.php(2 hunks)app/Http/Requests/UpdateRoomSettings.php(2 hunks)app/Http/Resources/RoomSettings.php(1 hunks)app/Http/Resources/RoomType.php(1 hunks)app/Models/Room.php(1 hunks)app/Models/RoomType.php(1 hunks)app/Services/MeetingService.php(1 hunks)database/migrations/2025_05_05_222245_add_dialin_pin_to_rooms_table.php(1 hunks)lang/de/rooms.php(2 hunks)lang/de/validation.php(2 hunks)lang/en/rooms.php(2 hunks)lang/en/validation.php(2 hunks)resources/js/components/RoomTabSettings.vue(2 hunks)resources/js/components/RoomTabSettingsDialinPinInput.vue(1 hunks)resources/js/composables/useRoomTypeSettings.js(1 hunks)resources/js/constants/roomSettings.js(1 hunks)resources/js/views/AdminRoomTypesView.vue(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
app/Services/MeetingService.php (1)
app/Models/Room.php (1)
getRoomSetting(412-435)
app/Http/Controllers/api/v1/RoomController.php (1)
app/Models/Room.php (1)
roomType(255-258)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Docker Build
🔇 Additional comments (30)
resources/js/constants/roomSettings.js (1)
53-55: Correctly implemented static dial-in PIN setting.The addition of the
dialin_pinsetting with the expert flag set to true is correct and follows the established pattern of the codebase.app/Http/Resources/RoomSettings.php (1)
34-34: Correctly exposed the dialin_pin field in the API response.The addition of the dialin_pin field to the resource's toArray method ensures it will be properly included in API responses.
app/Models/RoomType.php (1)
24-25: Appropriate boolean casts added for dial-in PIN room type settings.The addition of boolean casts for
has_dialin_pin_defaultandhas_dialin_pin_enforcedensures proper type conversion between database values and PHP boolean values.resources/js/composables/useRoomTypeSettings.js (1)
148-153: Correctly implemented dial-in PIN setting UI configuration.The dial-in PIN setting is properly added to the advanced settings group with the appropriate key, label, and switch type. This implementation follows the established pattern of the codebase.
resources/js/components/RoomTabSettings.vue (2)
132-132: Well-structured import for dialin PIN componentThe import of the new
RoomTabSettingsDialinPinInputcomponent follows the established pattern for other input components in this file.
338-343: Good implementation of the dial-in PIN input in advanced settingsThe new dial-in PIN setting is correctly integrated into the "advanced" section of the form. The implementation follows the same structure as other form items, using translation keys for labels and placeholders.
app/Services/MeetingService.php (1)
101-102: Good integration of dialin_pin as voiceBridge parameterThe implementation correctly sets the
voiceBridgeparameter using the room's dialin_pin setting. This ensures that the static PIN from room settings is passed to BigBlueButton during meeting creation.The use of
getRoomSetting()is appropriate as it handles the logic for enforced/default values from room types.app/Models/Room.php (1)
69-69: Appropriate casting for dialin_pin attributeThe dialin_pin is correctly cast as an integer, which is consistent with how other numeric identifiers like access_code are handled in this model.
app/Http/Resources/RoomType.php (1)
76-78: Correctly implemented room type settings for dialin PINThe addition of
has_dialin_pin_defaultandhas_dialin_pin_enforcedflags follows the established pattern for similar settings like access codes. These properties enable administrators to control whether the dialin PIN is enabled by default and whether it can be overridden at the room level.lang/en/validation.php (2)
83-83: Consistent label added for has_dialin_pin attributeThe new validation attribute for the
has_dialin_pinfield is properly added and follows the established naming convention pattern.
168-168: Consistent label added for dialin_pin attributeThe new validation attribute for the
dialin_pinfield is properly added and follows the established naming convention pattern.app/Http/Controllers/api/v1/RoomTypeController.php (2)
111-112: Properly added dialin PIN properties to the update methodThe dialin PIN default and enforced settings are correctly added to the update method, following the same pattern as the existing access code settings.
148-149: Properly added dialin PIN properties to the store methodThe dialin PIN default and enforced settings are correctly added to the store method, following the same pattern as the existing access code settings.
lang/de/validation.php (2)
83-83: Appropriate German translation for has_dialin_pin attributeThe translation for the
has_dialin_pinfield is properly added and uses an appropriate German term.
168-168: Appropriate German translation for dialin_pin attributeThe translation for the
dialin_pinfield is properly added and uses an appropriate German term.app/Http/Requests/RoomTypeRequest.php (2)
28-29: Properly added validation rules for dialin PIN settingsThe validation rules for the dialin PIN default and enforced settings are correctly added, following the same pattern as the existing access code settings.
55-56: Properly added attribute localization for dialin PIN settingsThe attribute localization for the dialin PIN settings is correctly implemented, using the validation translation system in a consistent manner.
resources/js/views/AdminRoomTypesView.vue (2)
1407-1451: Good implementation of the dial-in PIN UI sectionThe implementation follows the established UI pattern used for other settings in the application, with proper toggle controls for both default and enforced states. The translation keys are correctly referenced for internationalization support.
1610-1611: LGTM: Model properties correctly initializedThe reactive properties for the dial-in PIN feature are correctly added to the model object and initialized to
falseby default, matching the database column defaults.database/migrations/2025_05_05_222245_add_dialin_pin_to_rooms_table.php (2)
16-22: Sound database design for dial-in PIN functionalityThe migration properly adds boolean columns to the
room_typestable for feature control and a unique integer column to theroomstable for storing the actual PIN. Theuniqueconstraint ondialin_pinis appropriate for a feature that requires distinct values for different rooms.
29-36: Properly implemented down migrationThe
down()method correctly drops all added columns in case of rollback, maintaining database integrity.app/Http/Requests/UpdateRoomSettings.php (2)
20-20: LGTM: New validation rule properly addedThe dial-in PIN validation rule is correctly added to the rules array, using a dedicated method for rule generation similar to the pattern used for access codes.
78-78: Good validation constraints for dial-in PINThe validation ensures the PIN is a 5-digit integer between 10000-99999, which is a reasonable format for a dial-in PIN, and the uniqueness constraint prevents conflicts between different rooms.
lang/en/rooms.php (2)
5-5: LGTM: Top-level translation key addedThe main translation key for "Dialin PIN" is properly added to the language file.
362-367: Complete localization entries for dial-in PIN UI elementsAll necessary translation keys for the dial-in PIN feature UI elements have been added, following the existing pattern and terminology of the application.
app/Http/Controllers/api/v1/RoomController.php (2)
177-180: Implementation for dialog PIN generation looks goodThe logic correctly generates a random 5-digit PIN (between 11111 and 99999) when the room type has the
dialin_pin_defaultflag enabled. The approach aligns with the existing access code generation pattern.
281-281: LGTM! The room's dialin_pin update is properly handledThis change correctly adds support for updating the room's
dialin_pinattribute from the request input, consistent with how other settings are handled.lang/de/rooms.php (1)
5-5: LGTM! New translation for dialin_pinThe translation for "dialin_pin" is properly added, supporting the new feature in the German interface.
resources/js/components/RoomTabSettingsDialinPinInput.vue (2)
1-60: UI implementation looks clean and well-structuredThe component follows best practices with clear organization, proper data binding, and conditional rendering. The tooltip usage and accessibility attributes are implemented correctly.
62-105: Props and model definitions are well organizedThe component's script setup follows Vue 3 best practices with proper use of defineProps and defineModel. All props have appropriate types and default values where needed.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
|
Hello @q16marvin, Thank you for your PR. Unfortunately, adding static dial-in PINs is not that easy to accomplish. Scalelite can provide such a dial-in plan, but in PILOS you can also add the servers directly, so PILOS would also need to provide a dial-in plan. The combination of other load balancers and adding servers directly makes the whole thing even more complex, the dial in plans would have to be merged by something. In the event that other frontends can also create meetings on the same BBB server, we also need to ensure that the same number is not assigned twice, which is particularly difficult as BBB creates these numbers when they are not explicitly set. So it can happen that a meeting without a fixed PIN is assigned the same PIN by BBB as a room with a static PIN. If I am wrong somewhere, please correct me. It's been a few days since I last dealt with this complex issue. |
|
Hi @samuelwei, you are absolutly right, in big enviroments with some BBB Servers and maybe other frontends to manage this BBB Servers it can happen colliding PINs :( we have only one BBB Server and with a freeswitch dial in. Could we add this function with a .env setting called like "PILOS_DIALIN_PIN_ENABLE = true" and this stuff is only useable if this is true and document it with the "risky in big enviroments"? Or other Idea: BBB Servers generate Random PINs from 10000 - 99999. Pilos could use Pins from 00001 - 09999, than there would never be a conflict. thats the last "feature" why we cant move from our self developed stupid frontend to your extrem cool frontend for BBB. So i would be realy happy if we find any compromise :) |
88ee280 to
bc9287a
Compare
Fixes #
Type
Checklist
Changes
We have everyday on small meeting, if someone is on the road he must dialin via phone. Currently BBB create on every new Meeting a new random dialin pin. So for specified Meetings we need a fixed PIN that never changed.
This Change bringt a new Room Field:
Other information
Summary by CodeRabbit
New Features
Localization
Bug Fixes