admin setting to hide room owner on join of unauthenticated users#2844
admin setting to hide room owner on join of unauthenticated users#2844danielwujecki wants to merge 3 commits intoTHM-Health:developfrom
Conversation
WalkthroughThe PR implements a feature to hide room owner names from unauthenticated users through an admin-configurable toggle. Changes include backend settings configuration, database migration, API exposure, validation rules, and frontend UI for both admin controls and room display components. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Thanks for your PR @danielwujecki This fix is only hiding the name (visually), I would prefer hiding it from the API response. The frontend can check if the owner name is returned and show it. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@app/Http/Controllers/api/v1/SettingsController.php`:
- Line 186: The assignment to RoomSettings::$hide_owner_for_guests uses
$request->input(), which can return raw strings and not reliably cast to a PHP
bool; replace the call with $request->boolean('room_hide_owner_for_guests') in
the SettingsController where $roomSettings->hide_owner_for_guests is set so the
value is properly converted to a boolean consistent with the other boolean
fields.
In `@CHANGELOG.md`:
- Around line 8-11: The entry "Add admin setting to hide room owner on join of
unauthenticated users" under the "[Unreleased]" section is missing a category
header; add a "### Added" header directly beneath "## [Unreleased]" and place
that bullet under it so the changelog follows the Keep a Changelog format
(locate the "[Unreleased]" section and the line containing the bullet to move
it).
In `@lang/ar/admin.php`:
- Line 391: The translation for the 'room_hide_owner_for_guests' key is still in
English; replace its value in the lang/ar/admin.php localization array with the
Arabic translation (e.g., "إخفاء مالك الغرفة عن المستخدمين غير المسجلين") so the
string matches the rest of the Arabic locale and preserves the same array key.
In `@lang/fa/admin.php`:
- Line 389: Replace the English value for the translation key
'room_hide_owner_for_guests' with a Persian translation so the Farsi locale is
consistent; update the value for 'room_hide_owner_for_guests' in the
lang/fa/admin.php translations to a proper Farsi string (e.g., "مخفی کردن مالک
اتاق از کاربران احراز هویت نشده").
| $roomSettings->auto_delete_never_used_period = $request->enum('room_auto_delete_never_used_period', TimePeriod::class); | ||
| $roomSettings->auto_delete_deadline_period = $request->enum('room_auto_delete_deadline_period', TimePeriod::class); | ||
| $roomSettings->file_terms_of_use = $request->input('room_file_terms_of_use'); | ||
| $roomSettings->hide_owner_for_guests = $request->input('room_hide_owner_for_guests'); |
There was a problem hiding this comment.
Use $request->boolean() instead of $request->input() for a boolean setting.
Every other boolean field in this method uses $request->boolean() (e.g., Lines 171, 178, 188, 190). Using $request->input() returns the raw value (e.g., "1", "true", "0"), which may not properly cast to a PHP bool for the typed RoomSettings::$hide_owner_for_guests property.
Proposed fix
- $roomSettings->hide_owner_for_guests = $request->input('room_hide_owner_for_guests');
+ $roomSettings->hide_owner_for_guests = $request->boolean('room_hide_owner_for_guests');📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| $roomSettings->hide_owner_for_guests = $request->input('room_hide_owner_for_guests'); | |
| $roomSettings->hide_owner_for_guests = $request->boolean('room_hide_owner_for_guests'); |
🤖 Prompt for AI Agents
In `@app/Http/Controllers/api/v1/SettingsController.php` at line 186, The
assignment to RoomSettings::$hide_owner_for_guests uses $request->input(), which
can return raw strings and not reliably cast to a PHP bool; replace the call
with $request->boolean('room_hide_owner_for_guests') in the SettingsController
where $roomSettings->hide_owner_for_guests is set so the value is properly
converted to a boolean consistent with the other boolean fields.
| ## [Unreleased] | ||
|
|
||
| - Add admin setting to hide room owner on join of unauthenticated users | ||
|
|
There was a problem hiding this comment.
Missing ### Added category header under [Unreleased].
All other version sections follow the Keep a Changelog format with category headers. This entry should be placed under ### Added.
Proposed fix
## [Unreleased]
+### Added
+
- Add admin setting to hide room owner on join of unauthenticated users📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ## [Unreleased] | |
| - Add admin setting to hide room owner on join of unauthenticated users | |
| ## [Unreleased] | |
| ### Added | |
| - Add admin setting to hide room owner on join of unauthenticated users |
🤖 Prompt for AI Agents
In `@CHANGELOG.md` around lines 8 - 11, The entry "Add admin setting to hide room
owner on join of unauthenticated users" under the "[Unreleased]" section is
missing a category header; add a "### Added" header directly beneath "##
[Unreleased]" and place that bullet under it so the changelog follows the Keep a
Changelog format (locate the "[Unreleased]" section and the line containing the
bullet to move it).
lang/ar/admin.php
Outdated
| 'description' => 'الفترة منذ آخر استخدام التي يتم بعدها حذف روابط الغرف المخصصة تلقائيًا.', | ||
| 'title' => 'مدة صلاحية روابط الغرف المخصصة', | ||
| ], | ||
| 'room_hide_owner_for_guests' => 'Hide the room owner from unauthenticated users', |
There was a problem hiding this comment.
Translation string is not localized — still in English.
Same issue as the Farsi locale: this string should be translated to Arabic for consistency with the rest of the file (e.g., "إخفاء مالك الغرفة عن المستخدمين غير المسجلين").
🤖 Prompt for AI Agents
In `@lang/ar/admin.php` at line 391, The translation for the
'room_hide_owner_for_guests' key is still in English; replace its value in the
lang/ar/admin.php localization array with the Arabic translation (e.g., "إخفاء
مالك الغرفة عن المستخدمين غير المسجلين") so the string matches the rest of the
Arabic locale and preserves the same array key.
lang/fa/admin.php
Outdated
| 'description' => 'مدت زمان از آخرین استفاده، پس از آن لینکهای شخصیسازیشده اتاق بهطور خودکار حذف میشوند.', | ||
| 'title' => 'زمان انقضا برای لینکهای شخصیسازیشده اتاق', | ||
| ], | ||
| 'room_hide_owner_for_guests' => 'Hide the room owner from unauthenticated users', |
There was a problem hiding this comment.
Translation string is not localized — still in English.
All other entries in this Farsi locale file are translated into Persian, but this new key remains in English. This should be translated to Farsi for consistency (e.g., something like "مخفی کردن مالک اتاق از کاربران احراز هویت نشده").
🤖 Prompt for AI Agents
In `@lang/fa/admin.php` at line 389, Replace the English value for the translation
key 'room_hide_owner_for_guests' with a Persian translation so the Farsi locale
is consistent; update the value for 'room_hide_owner_for_guests' in the
lang/fa/admin.php translations to a proper Farsi string (e.g., "مخفی کردن مالک
اتاق از کاربران احراز هویت نشده").
|
Please remove all non-english locale changes, as they will be removed. All non-english locales are synced from PoEditor (one-way) |
danielwujecki
force-pushed
the
2843-admin-setting-hide-room-owner-for-guests
branch
from
0e80acb to
8fc332c
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
resources/js/components/RoomDetailsList.vue (1)
7-14:⚠️ Potential issue | 🟠 MajorFrontend-only hiding is insufficient — owner data is still exposed in the API response.
As noted by reviewer samuelwei, the owner's name is still returned by the API and visible in network responses / browser dev tools. The backend should omit
owner.namefrom the API response for unauthenticated users when the setting is enabled, and the frontend should conditionally render based on whether the field is present in the response rather than a separate flag.This approach would also simplify this component — no need for the
showRoomOwnerprop if the API simply doesn't return the owner data.
🤖 Fix all issues with AI agents
In `@lang/de/admin.php`:
- Line 391: Remove the manual non-English locale entry by deleting the
'room_hide_owner_for_guests' => 'Eigentümer eines Raumes für Gäste verstecken'
array element from lang/de/admin.php so translations remain managed through
PoEditor; ensure no other manual German locale keys were added in the same file
and leave only keys that are synced from the canonical source or empty
placeholders if required by the loader.
lang/de/admin.php
Outdated
| 'description' => 'Zeitraum ab der letzten Nutzung, wonach die personalisierten Raumlinks automatisch gelöscht werden.', | ||
| 'title' => 'Ablaufzeit für personalisierte Raumlinks', | ||
| ], | ||
| 'room_hide_owner_for_guests' => 'Eigentümer eines Raumes für Gäste verstecken', |
There was a problem hiding this comment.
Remove non-English locale changes — managed via PoEditor.
Per reviewer feedback, non-English locales are managed one-way via PoEditor. This addition will be overwritten or cause conflicts. Remove this change and let the translation be handled through the PoEditor workflow.
🤖 Prompt for AI Agents
In `@lang/de/admin.php` at line 391, Remove the manual non-English locale entry by
deleting the 'room_hide_owner_for_guests' => 'Eigentümer eines Raumes für Gäste
verstecken' array element from lang/de/admin.php so translations remain managed
through PoEditor; ensure no other manual German locale keys were added in the
same file and leave only keys that are synced from the canonical source or empty
placeholders if required by the loader.
danielwujecki
force-pushed
the
2843-admin-setting-hide-room-owner-for-guests
branch
from
8fc332c to
719404b
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
app/Http/Resources/Room.php (1)
86-97: Good approach: owner data is properly excluded from the API response.This correctly addresses the reviewer feedback to remove the owner from the API response rather than just hiding it in the frontend.
One minor note: the array on lines 95–96 is eagerly evaluated even when
$showOwneris false. Consider using a closure to defer evaluation:♻️ Suggested improvement
- 'owner' => $this->when($showOwner, [ - 'id' => $this->owner->id, - 'name' => $this->owner->fullname, - ]), + 'owner' => $this->when($showOwner, fn () => [ + 'id' => $this->owner->id, + 'name' => $this->owner->fullname, + ]),Since the
ownerrelationship is likely already eager-loaded, this is a minor optimization, but it avoids unnecessary property access when the value will be discarded.
Done
Good point, I improved this. |
|
|
||
| ## [Unreleased] | ||
|
|
||
| - Add admin setting to hide room owner on join of unauthenticated users |
There was a problem hiding this comment.
Add subheading according to keepachangelog and add links to issue and PR
| }); | ||
|
|
||
| const emit = defineEmits(["joinedMembership", "reload", "invalidCode"]); | ||
|
|
| binary | ||
| :disabled="disabled" | ||
| :invalid=" | ||
| formErrors.fieldInvalid('room_hide_owner_for_guests') |
There was a problem hiding this comment.
Field room_hide_owner_for_guests should be added to lang/en/validation.php
Please also add this to the frontend cypress tests
| $roomSettings->auto_delete_never_used_period = $request->enum('room_auto_delete_never_used_period', TimePeriod::class); | ||
| $roomSettings->auto_delete_deadline_period = $request->enum('room_auto_delete_deadline_period', TimePeriod::class); | ||
| $roomSettings->file_terms_of_use = $request->input('room_file_terms_of_use'); | ||
| $roomSettings->hide_owner_for_guests = $request->input('room_hide_owner_for_guests'); |
| $roomSettings = app(RoomSettings::class); | ||
|
|
||
| // Check if user is authenticated or room owner should be shown to everyone | ||
| $showOwner = Auth::check() || !$roomSettings->hide_owner_for_guests; | ||
|
|
||
| return [ | ||
| 'id' => $this->id, | ||
| 'name' => $this->name, | ||
| 'owner' => [ | ||
| 'owner' => $this->when($showOwner, [ |
| > | ||
| <!--owner name--> | ||
| <div class="flex"> | ||
| <div v-if="props.room.owner" class="flex"> |
|
Please also run the code style fixers, see https://thm-health.github.io/PILOS/docs/development/code-style |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #2844 +/- ##
=============================================
+ Coverage 96.77% 97.27% +0.50%
=============================================
Files 440 177 -263
Lines 12479 6277 -6202
Branches 2073 2074 +1
=============================================
- Hits 12076 6106 -5970
+ Misses 403 171 -232 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
2 participants
Fixes #2843
Type
Checklist
Changes
room.hide_owner_for_gueststoRoomSettings.AdminSettings.vueunder room settings.RoomHeader.vueandRoomDetailsList.vueto hide the owner's name for unauthenticated users if the setting is enabled.Config.phpandSettings.phpresources to expose the setting to the frontend.Other information
How to test:
Summary by CodeRabbit