feat: Privacy setting to disable finding users by partial matches of their name - #3316
feat: Privacy setting to disable finding users by partial matches of their name#3316samuelwei wants to merge 16 commits into
Conversation
|
Warning Review limit reached
Next review available in: 12 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
WalkthroughAdds an administrator-controlled ChangesUser search privacy setting
Estimated code review effort: 4 (Complex) | ~45 minutes Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 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 |
|
|
@Pierre-Lannoy Is that resolving your issue ? |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #3316 +/- ##
=============================================
- Coverage 97.10% 97.10% -0.01%
- Complexity 1949 1950 +1
=============================================
Files 483 484 +1
Lines 16656 16666 +10
Branches 2408 2409 +1
=============================================
+ Hits 16174 16183 +9
- Misses 482 483 +1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
PILOS
 |
||||||||||||||||||||||||||||||||||
| Project |
PILOS
|
| Branch Review |
2264-disable-find-users-by-name
|
| Run status |
|
| Run duration | 07m 57s |
| Commit |
|
| Committer | Samuel Weirich |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
 |
1
|
 |
0
|
 |
0
|
 |
0
|
 |
636
|
| View all changes introduced in this branch ↗︎ | |
Tests for review
| Test | Artifacts | |
|---|---|---|
| Room Join with lobby settings > Lobby enabled |
Test Replay
Screenshots
|
|
|
Hello @samuelwei ! |
Sabr1n4W
left a comment
Sabr1n4W
left a comment
There was a problem hiding this comment.
Not completely sure about debouncing the user search requests.
The debounce itself makes sense to reduce the number of user search requests but especially the error messages shown while typing (Too many users found, No user found) can be a bit confusing.
e.g. the following scenario:
- Open add single user dialog
- Enter a name that produces no results.
- Error message "Oops! No user was found for this query" shown while typing
- Clear name and wait for request with empty query
- Enter the same name again
- Error message "To many users found..." shown while typing
Not sure if the error messages could/should maybe be improved to be less confusing or maybe be hidden while typing.
| ->assertSuccessful() | ||
| ->assertJsonCount(0, 'data'); | ||
|
|
||
| // Check with exact lastname match (search for names should not be possible) |
There was a problem hiding this comment.
| // Check with exact lastname match (search for names should not be possible) | |
| // Check with exact firstname match (search for names should not be possible) |
| "rooms.members.modals.add.no_options_email_only", | ||
| ); | ||
|
|
||
| // Check placeholder and type in input |
There was a problem hiding this comment.
| // Check placeholder and type in input | |
| // Check placeholder |
| "rooms.members.modals.add.no_options_email_only", | ||
| ); | ||
|
|
||
| // Check placeholder and type in input |
There was a problem hiding this comment.
| // Check placeholder and type in input | |
| // Check placeholder |
| * @return AnonymousResourceCollection | ||
| */ | ||
| public function search(UserSearchRequest $request) | ||
| public function search(UserSearchRequest $request, UserSettings $userSettings) |
There was a problem hiding this comment.
Not completely sure about this
Currently we mostly use app(UserSettings::class) etc. to access the settings. The way used here is also working and from what I understand might be the cleaner way to do this so I would tend to keep it.
If this is kept, the docblock should perhaps be updated accordingly.
Also the new search behavior should be included/described in the docblock.
There was a problem hiding this comment.
- change to app(...) syntax for now
- refactor everything to dependency injection in a new PR
|
|
||
| const onSearchChange = useDebounceFn((query) => { | ||
| asyncFind(query); | ||
| }, 500); |
There was a problem hiding this comment.
Unsure what a good debounce time would be. Research is unclear, somewhere between 200 - 500 ms.
Added ms query logging to our instance on 23.07.2026 10:40 (Europe/Berlin), will analyse logs after a week to find min./avg./med. input time for our users
|
Current results: |
There was a problem hiding this comment.
Pull request overview
Adds a new instance-wide privacy setting that controls whether users can be found via partial name/email matching, wiring it through backend settings/config APIs and updating affected frontend member/ownership transfer search UX accordingly.
Changes:
- Introduces
user.search_by_namesetting (defaulttrue) and exposes it via/api/v1/settingsand/api/v1/config. - Updates user search behavior: when disabled, backend search only matches exact email instead of partial name/email matching.
- Updates admin UI + member/ownership-transfer UI strings and Cypress coverage (including debounced searching).
Reviewed changes
Copilot reviewed 21 out of 21 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/Frontend/fixtures/settings.json | Adds user_search_by_name to settings fixture. |
| tests/Frontend/fixtures/config.json | Adds user.search_by_name to config fixture. |
| tests/Frontend/e2e/RoomsViewSettings.cy.js | Updates ownership transfer placeholder expectations; adds “search by name disabled” scenario. |
| tests/Frontend/e2e/RoomsViewMembersMemberActions.cy.js | Updates member-add search assertions; adds “search by name disabled” scenario. |
| tests/Frontend/e2e/AdminSettingsView.cy.js | Verifies new admin setting is displayed in view-only mode. |
| tests/Frontend/e2e/AdminSettingsEdit.cy.js | Verifies new admin setting can be edited and is sent in form payload. |
| tests/Backend/Feature/api/v1/UserTest.php | Adds backend coverage for search behavior when the setting is disabled. |
| tests/Backend/Feature/api/v1/SettingsTest.php | Extends settings API tests to include the new setting (but contains a naming bug). |
| resources/js/views/AdminSettings.vue | Adds toggle UI for the new setting in Admin Settings. |
| resources/js/components/RoomTransferOwnershipButton.vue | Uses config setting to adjust placeholder/prompt text; debounces search requests. |
| resources/js/components/RoomTabMembersAddSingleModal.vue | Uses config setting to adjust placeholder/prompt text; debounces search requests. |
| lang/en/rooms.php | Adds new i18n strings for email-only prompt/placeholder. |
| lang/en/admin.php | Adds i18n title/description for the admin setting. |
| database/settings/2026_07_15_130614_add_user_name_search_disabled.php | Adds the new user.search_by_name setting (default true). |
| CHANGELOG.md | Documents the feature and adds link references (one link target is incorrect). |
| app/Settings/UserSettings.php | Adds public bool $search_by_name. |
| app/Http/Resources/SettingsResource.php | Exposes user_search_by_name via SettingsResource. |
| app/Http/Resources/ConfigResource.php | Exposes user.search_by_name via ConfigResource. |
| app/Http/Requests/UpdateSettingsRequest.php | Validates user_search_by_name as a required boolean. |
| app/Http/Controllers/api/v1/UserController.php | Switches user search query logic based on search_by_name. |
| app/Http/Controllers/api/v1/SettingsController.php | Persists user_search_by_name into UserSettings::search_by_name. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/Http/Controllers/api/v1/UserController.php`:
- Around line 52-62: Update the search method PHPDoc to describe both supported
behaviors: name-or-email searching when UserSettings::search_by_name is enabled
and email-only searching otherwise. Keep the documentation aligned with the
existing User::withNameOrEmail and User::whereLike branches.
- Around line 52-62: Update the search_by_name-disabled branch in UserController
to perform an exact, case-insensitive email lookup instead of passing raw input
to User::whereLike, ensuring % and _ remain literal characters. Preserve the
existing name-or-email search behavior and add regression coverage for email
queries containing both wildcard characters.
In `@resources/js/components/RoomTabMembersAddSingleModal.vue`:
- Around line 192-200: Prevent stale search results and loading updates by
adding a monotonically increasing search generation to the debounced/API flow in
resources/js/components/RoomTabMembersAddSingleModal.vue lines 192-200 and
resources/js/components/RoomTransferOwnershipButton.vue lines 282-290. Pass the
generation through each search, apply results and loading-state changes only
when it is still current, and invalidate or cancel the generation when the modal
closes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: d0139e97-78ec-40fb-8b04-a24cbb3927ca
📒 Files selected for processing (21)
CHANGELOG.mdapp/Http/Controllers/api/v1/SettingsController.phpapp/Http/Controllers/api/v1/UserController.phpapp/Http/Requests/UpdateSettingsRequest.phpapp/Http/Resources/ConfigResource.phpapp/Http/Resources/SettingsResource.phpapp/Settings/UserSettings.phpdatabase/settings/2026_07_15_130614_add_user_name_search_disabled.phplang/en/admin.phplang/en/rooms.phpresources/js/components/RoomTabMembersAddSingleModal.vueresources/js/components/RoomTransferOwnershipButton.vueresources/js/views/AdminSettings.vuetests/Backend/Feature/api/v1/SettingsTest.phptests/Backend/Feature/api/v1/UserTest.phptests/Frontend/e2e/AdminSettingsEdit.cy.jstests/Frontend/e2e/AdminSettingsView.cy.jstests/Frontend/e2e/RoomsViewMembersMemberActions.cy.jstests/Frontend/e2e/RoomsViewSettings.cy.jstests/Frontend/fixtures/config.jsontests/Frontend/fixtures/settings.json
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 21 out of 21 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (2)
app/Http/Controllers/api/v1/UserController.php:64
- Typo in comment: “wild-card” is usually written as “wildcard”.
// Escape wild-card characters and escape backslashes to prevent escaping our added escape backslash
app/Http/Controllers/api/v1/UserController.php:48
- Docblock grammar: “search of partial matches” should be “search for partial matches” to accurately read as a sentence.
This issue also appears on line 64 of the same file.
* Search for users in the whole database
* Depending on the search_by_name setting it will either search of partial matches of firstname, lastname and email
* or only for an exact email match
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 21 out of 21 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (6)
tests/Backend/Feature/api/v1/UserTest.php:279
- This comment says "exact lastname match" but the query used is
Gregory(firstname). This makes the test harder to understand/maintain.
// Check with exact lastname match (search for names should not be possible)
app/Http/Controllers/api/v1/UserController.php:48
- Docblock grammar: "search of partial matches" should be "search for partial matches" to accurately describe the behavior.
* Search for users in the whole database
* Depending on the search_by_name setting it will either search for partial matches of firstname, lastname and email
* or only for an exact email match
tests/Backend/Feature/api/v1/UserTest.php:274
- This comment says "exact firstname match" but the query used is
Dumas(lastname). This makes the test harder to understand/maintain.
This issue also appears on line 279 of the same file.
// Check with exact firstname match (search for names should not be possible)
resources/js/components/RoomTransferOwnershipButton.vue:287
onSearchChange()clearsusersand sets loading, but it doesn't resettooManyResults. If the previous query produced a 204 "too many results", the UI can briefly show the wrong message for subsequent queries until the next response arrives.
function onSearchChange(query) {
isLoadingSearch.value = true;
users.value = [];
debouncedFind(query);
}
resources/js/components/RoomTabMembersAddSingleModal.vue:197
onSearchChange()clearsusersand sets loading, but it doesn't resettooManyResults. If the previous query produced a 204 "too many results", the UI can briefly show the wrong message for subsequent queries until the next response arrives.
function onSearchChange(query) {
isLoadingSearch.value = true;
users.value = [];
debouncedFind(query);
}
database/settings/2026_07_15_130614_add_user_name_search_disabled.php:18
- The settings migration filename suggests the feature is about "user name search disabled", but the setting being added is
user.search_by_namewith a default oftrue(enabled). Renaming the migration file to match the actual setting intent will make the settings history easier to follow.
<?php
declare(strict_types=1);
use Spatie\LaravelSettings\Migrations\SettingsMigration;
return new class extends SettingsMigration
{
public function up(): void
{
$this->migrator->add('user.search_by_name', true);
}
public function down(): void
{
$this->migrator->delete('user.search_by_name');
}
};
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 21 out of 21 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (6)
tests/Backend/Feature/api/v1/UserTest.php:279
- The comment says this is a lastname match, but the query being used is the user's firstname (
Gregory). This makes the test harder to understand/maintain.
// Check with exact lastname match (search for names should not be possible)
resources/js/components/RoomTransferOwnershipButton.vue:287
tooManyResultsand the in-flight request aren't reset/cancelled when a new search starts. With the 300ms debounce, an older request can still resolve and repopulate the dropdown with stale results (and the UI can keep showing the “too many results” message) until the next request runs.
function onSearchChange(query) {
isLoadingSearch.value = true;
users.value = [];
debouncedFind(query);
}
resources/js/components/RoomTabMembersAddSingleModal.vue:197
tooManyResultsand the previous in-flight request aren't reset/cancelled when a new search starts. Because the request is only aborted insideasyncFind(after the debounce delay), the prior request can still resolve and briefly show stale results for the previous query.
function onSearchChange(query) {
isLoadingSearch.value = true;
users.value = [];
debouncedFind(query);
}
tests/Backend/Feature/api/v1/UserTest.php:262
- Test name contains a duplicated word (
test_search_search_…). Renaming it improves readability while keeping PHPUnit discovery (method still starts withtest_).
public function test_search_search_by_name_disabled()
tests/Backend/Feature/api/v1/UserTest.php:274
- The comment says this is a firstname match, but the query being used is the user's lastname (
Dumas). This makes the test harder to understand/maintain.
This issue also appears on line 279 of the same file.
// Check with exact firstname match (search for names should not be possible)
database/settings/2026_07_15_130614_add_user_name_search_disabled.php:11
- The migration filename suggests this adds a disabled setting (
add_user_name_search_disabled), but the migration actually addsuser.search_by_namewith a default oftrue. Renaming the migration file to match the setting/purpose would avoid confusion when scanning migration history.
$this->migrator->add('user.search_by_name', true);
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 21 out of 21 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (4)
resources/js/components/RoomTransferOwnershipButton.vue:287
onSearchChangeclearsusersbut doesn’t resettooManyResults, so a previous “too many results” state can be shown while the debounced request is pending. Also, because the request is only aborted insideasyncFind, stale results from the previous in-flight request can still resolve during the 300ms debounce window and overwrite the list.
function onSearchChange(query) {
isLoadingSearch.value = true;
users.value = [];
debouncedFind(query);
}
resources/js/components/RoomTabMembersAddSingleModal.vue:197
onSearchChangeclearsusersbut doesn’t resettooManyResults, so the “too many results” message can persist from a previous search while the new debounced request is pending. Also, aborting only insideasyncFindmeans an older in-flight request can still resolve during the debounce delay and overwrite the results.
function onSearchChange(query) {
isLoadingSearch.value = true;
users.value = [];
debouncedFind(query);
}
lang/en/rooms.php:228
- New i18n keys (
no_options_email_only,placeholder_email_only) were added only to the English locale. Since otherlang/*/rooms.phpfiles exist and don’t contain these keys, users on those locales will likely see the raw translation key in the UI when name search is disabled.
'no_options' => 'Enter the name or email address of a user.',
'no_options_email_only' => 'Enter the email address of a user.',
'no_result' => 'Oops! No user was found for this query.',
'placeholder' => 'Name or email',
'placeholder_email_only' => 'Email',
lang/en/admin.php:432
- The new
admin.settings.user_search_by_name.*translation keys were added only inlang/en/admin.php. Other locales (lang/*/admin.php) exist and don’t include these keys, so non-English admins may see untranslated keys in the settings UI.
'user_search_by_name' => [
'description' => 'Allows users to find others by partial matches of their name or email address. When disabled, users can only be found by entering their exact email address.',
'title' => 'Allow searching users by name',
],
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 22 out of 22 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (7)
resources/js/components/RoomTransferOwnershipButton.vue:168
usersis now unused after switching toUserSearch(it manages its own options list). Keeping this ref increases confusion and can drift out of sync with the UI.
This issue also appears on line 220 of the same file.
const isLoadingAction = ref(false);
const modalVisible = ref(false);
const users = ref([]);
const newOwner = ref(null);
const newRoleInRoom = ref(3);
resources/js/components/RoomTransferOwnershipButton.vue:225
- This reset is for the now-removed
usersref and can be deleted onceusersis removed.
function showModal() {
newOwner.value = null;
users.value = [];
newRoleInRoom.value = 3;
formErrors.clear();
modalVisible.value = true;
resources/js/components/UserSearch.vue:95
- Abort any in-flight request when
UserSearchunmounts to avoid updating state after unmount and to prevent unnecessary network work.
let abortController = new AbortController();
function onSearchChange(query) {
resources/js/components/RoomTransferOwnershipButton.vue:38
- Room transfer ownership: the user picker no longer receives the invalid state, so validation errors for
userwon't visually mark the input as invalid even though a FormError is shown. Pass the invalid prop through toUserSearchlike the other member dialogs do.
<UserSearch
v-model="newOwner"
aria-labelledby="user-label"
data-test="new-owner-dropdown"
:disabled="isLoadingAction"
/>
resources/js/components/RoomTransferOwnershipButton.vue:38
- Room transfer ownership: removing the local
asyncFind()logic means the current owner is no longer marked as disabled in the dropdown, so users can select an invalid target and only learn on submit (TransferOwnershipRequest rejects selecting the current owner). Consider adding support inUserSearchfor disabling/filtering specific user IDs (e.g. the current owner) and use it here.
<UserSearch
v-model="newOwner"
aria-labelledby="user-label"
data-test="new-owner-dropdown"
:disabled="isLoadingAction"
/>
resources/js/components/RoomTabMembersAddSingleModal.vue:133
- After switching to
UserSearch, the localusersref is unused and the corresponding reset inshowModal()is dead code. Removing both avoids confusion and keeps the state minimal.
const user = ref(null);
const role = ref(null);
const users = ref([]);
const isLoadingAction = ref(false);
resources/js/components/UserSearch.vue:63
UserSearchperforms async requests; add an unmount hook so in-flight requests can be canceled when the component is destroyed.
This issue also appears on line 93 of the same file.
import Multiselect from "vue-multiselect";
import { ref } from "vue";
import { useApi } from "../composables/useApi.js";
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
resources/js/components/UserSearch.vue (1)
2-2: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoff
vue-multiselectused instead of a PrimeVue component.This new shared component is built on
vue-multiselectrather than a PrimeVue component (e.g.AutoComplete/Select). This mirrors the prior inline implementation, but the coding guideline forresources/js/{views,components}/**/*.vuecalls for PrimeVue-based UI for design consistency.As per coding guidelines,
resources/js/{views,components}/**/*.vue: "Use PrimeVue components for building the UI within a Vue.js SPA instead of custom UI components". Since this is a lift of an existing established pattern rather than a new decision, please confirm whether a PrimeVue-based replacement (e.g.,AutoComplete) was considered or is planned for a follow-up.Also applies to: 61-61
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@resources/js/components/UserSearch.vue` at line 2, Replace the vue-multiselect instance in UserSearch with a PrimeVue component, preferably AutoComplete or Select, while preserving the existing user-search behavior and shared component API. Remove the vue-multiselect-specific usage and adapt its bindings, options, and selection handling to the chosen PrimeVue component.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@resources/js/components/UserSearch.vue`:
- Line 67: Update the defineModel declaration in UserSearch.vue to explicitly
type model as the selected user object or null, using the existing user type
symbol if available. Preserve the current model behavior and default value.
---
Nitpick comments:
In `@resources/js/components/UserSearch.vue`:
- Line 2: Replace the vue-multiselect instance in UserSearch with a PrimeVue
component, preferably AutoComplete or Select, while preserving the existing
user-search behavior and shared component API. Remove the
vue-multiselect-specific usage and adapt its bindings, options, and selection
handling to the chosen PrimeVue component.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 8960f7fb-1628-4e94-a67f-89c83d4b1b66
📒 Files selected for processing (4)
lang/en/rooms.phpresources/js/components/RoomTabMembersAddSingleModal.vueresources/js/components/RoomTransferOwnershipButton.vueresources/js/components/UserSearch.vue
🚧 Files skipped from review as they are similar to previous changes (2)
- lang/en/rooms.php
- resources/js/components/RoomTransferOwnershipButton.vue
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 22 out of 22 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (5)
tests/Backend/Feature/api/v1/UserTest.php:306
- The query string here includes a raw backslash and '%'. Both characters should be URL-encoded when building the test request; otherwise parsing can differ from real client behavior and may vary by server/proxy. Using http_build_query also makes it explicit you’re testing literal handling of these characters after URL decoding.
$this->actingAs($users[0])->getJson(route('api.v1.users.search').'?query=gregory\%')
->assertSuccessful()
->assertJsonCount(0, 'data');
tests/Backend/Feature/api/v1/UserTest.php:314
- Same URL-encoding issue as above: this request includes a raw backslash and '%'. Encoding via http_build_query ensures the test is asserting behavior after proper URL decoding (what a browser/client would send).
$this->actingAs($users[0])->getJson(route('api.v1.users.search').'?query=gregory\%')
->assertSuccessful()
->assertJsonCount(0, 'data');
tests/Backend/Feature/api/v1/UserTest.php:322
- This query string includes a raw backslash and '%' (both should be URL-encoded). Using http_build_query here keeps the test aligned with real client behavior while still verifying that the backend treats escaped wildcard characters literally.
$this->actingAs($users[0])->getJson(route('api.v1.users.search').'?query=gregory\_%.dumas@example.com')
->assertSuccessful()
->assertJsonCount(1, 'data');
lang/en/rooms.php:229
- The translation for the loading state uses spaced ellipsis ("Searching ..."), which is inconsistent with typical punctuation and other UI strings. Consider changing to "Searching..." for a cleaner message.
'searching' => 'Searching ...',
tests/Backend/Feature/api/v1/UserTest.php:298
- These requests put a raw '%' in the query string. In URLs, '%' should be percent-encoded (otherwise it can be interpreted as the start of an escape sequence), so the test is relying on permissive parsing. Build the query string with http_build_query/urlencode so the request matches real client behavior while still asserting that '%' is treated literally by the backend.
This issue also appears in the following locations of the same file:
- line 304
- line 312
- line 320
$this->actingAs($users[0])->getJson(route('api.v1.users.search').'?query=gregory%')
->assertSuccessful()
->assertJsonCount(0, 'data');
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 22 out of 22 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
resources/js/components/RoomTransferOwnershipButton.vue:39
- Selecting the current room owner as the “new owner” is no longer prevented in the dropdown (previously the owner option was marked as disabled). The backend correctly rejects this in TransferOwnershipRequest, but this is a UX regression that will now surface as a validation error that could be avoided client-side.
<div class="field relative mt-2 flex flex-col gap-2 overflow-visible">
<label id="user-label">{{ $t("app.user") }}</label>
<UserSearch
v-model="newOwner"
:disabled="isLoadingAction"
:invalid="formErrors.fieldInvalid('user')"
aria-labelledby="user-label"
data-test="new-owner-dropdown"
/>
Fixes #2264
Type
Checklist
Changes
Summary by CodeRabbit