fix(settings): trigger group search on the correct NcSelect event (#7988) by ethanhawkes-gif · Pull Request #7989 · LibreSign/libresign

ethanhawkes-gif · 2026-08-17T02:04:09Z

Summary

Fixes #7988. In Settings → Signature request access, typing in the group
selector did not trigger a backend search, so on instances with more than 20
groups it was impossible to find/select groups beyond the first (limit‑20) page.

Root cause

src/views/Settings/AllowedGroups.vue listened for @search-change on
<NcSelect>:

@search-change="searchGroup"

NcSelect (from @nextcloud/vue, ^9.9.0) does not emit a search-change
event. It re-exposes vue-select's native search event (internally
@search="search = $event", and its emits list only declares
update:modelValue). Because search-change is never emitted, searchGroup
was only ever called once from onMounted, and user input triggered no request
— exactly the reported symptom.

Fix

Listen for the correct event:

@search="searchGroup"

Vue 3 fallthrough merges this listener with NcSelect's internal @search
handler, so searchGroup(query) now runs on input and queries
cloud/groups/details?search=<query>. The existing selection/save flow
(@update:modelValue="saveGroups") is untouched.

Tests

Adds a regression test to src/tests/views/Settings/AllowedGroups.spec.ts that
emits the search event and asserts a cloud/groups/details request is issued
with the typed query.

npx vitest run src/tests/views/Settings/AllowedGroups.spec.ts → 5 passed
The new test fails on the previous @search-change wiring (zero requests
after typing) and passes with the fix, so it guards against reintroduction.
No new lint errors introduced by the change.

The "Signature request access" group selector listened for `@search-change`, which NcSelect (from @nextcloud/vue) / vue-select does not emit. Typing in the selector therefore never called `searchGroup`, so only the initial `onMounted` load (limit 20) ran and groups beyond the first 20 could not be found on larger instances. NcSelect exposes vue-select's native `search` event; switching the listener to `@search` makes typing query the backend as intended. The existing selection/save flow (`@update:modelValue`) is unchanged. Adds a regression test asserting that emitting the `search` event issues a `cloud/groups/details` request with the typed query. The test fails on the previous `@search-change` wiring (zero requests) and passes with the fix. Fixes LibreSign#7988 Signed-off-by: Ethan Hawkes <ethanhawkes-gif@users.noreply.github.com>

Spitfireap · 2026-08-17T08:50:41Z

Note : this should be backported to other stables when ready.

Tested on a dev instance (finally managed to set it up). It does work but the UX is not so great, the search input loses focus at every loading changes so basically for every character.

Perhaps a debounce would improve it ? Other possibility I see is to target only the select list sub-component for the loading state when searching. That way it won't disable the search input while searching.

Current behavior (please ignore the console.log, it is a test on my side) :
Enregistrement d'écran_20260817_104223.webm

…#7988) Follow-up to the `@search` wiring fix. On every keystroke `searchGroup` set `loadingGroups = true`, which is bound to NcSelect's `:disabled`. NcSelect propagates `disabled` to its internal `vs__search` input, and disabling a focused input fires `blur`, so the search field lost focus on each character typed (reported on LibreSign#7989). Scope the search loading state to the list sub-component instead: - A `searching` ref drives only `:loading` (the spinner), never `:disabled`, so the input keeps focus while typing. - Debounce the backend request (300ms) so typing issues a single `cloud/groups/details` call for the final query instead of one per keystroke. - Guard against out-of-order responses with a monotonic `searchSeq` token, and drop late responses after unmount via an `active` flag; `onBeforeUnmount` clears the pending debounce timer. - `onMounted` loads via the un-debounced `fetchGroups('')` and releases `loadingGroups` in a `finally`. Adds tests for the debounce + focus preservation (input stays enabled, three keystrokes collapse to one request) and for the out-of-order guard (a superseded slow response does not clobber a newer search). Signed-off-by: Ethan Hawkes <ethanhawkes-gif@users.noreply.github.com>

ethanhawkes-gif · 2026-08-17T09:39:15Z

Thanks for setting up the dev instance and testing this — and especially for the screen recording, it pinned the mechanism exactly.

You had the cause right: searchGroup flipped loadingGroups on every keystroke, and that ref is bound to NcSelect's :disabled. NcSelect passes disabled down to its internal vs__search input, so each character briefly disabled a focused input and the browser blurred it.

Pushed a follow-up commit (47b0f23) that applies both of your suggestions:

The per-search loading state now drives only :loading (the spinner on the list sub-component), never :disabled — so the input keeps focus while you type.
The backend search is debounced (300 ms): typing finance now makes a single cloud/groups/details call for the final query instead of one per character, with a monotonic guard so a slower earlier response can't overwrite a newer one (plus a pending-timer cleanup on unmount).

To re-test: pull the branch and rebuild the front-end (npm ci && npm run build), then open Signature request access and type in the group selector — focus should stay in the field the whole time, and the list should refresh once you pause. New unit tests cover the debounce, focus preservation (the input stays enabled), and the out-of-order guard.

Agreed on backporting to the stables — this and the original @search fix should travel together once this lands.

One thing, purely for our own before/after notes: roughly how much time was the stuck selector costing you per group lookup before this fix (all the re-clicking and retyping) versus now — even a rough "a few seconds each time" is plenty?

Spitfireap · 2026-08-17T09:55:32Z

Are you an AI ?

Anyway, I think that 47b0f23 is way too complicated. You could simply use the loading object passed to the search event.
Here is what I suggest (diff from your first commit) :
libresign.patch
Please include the co-author tag if you're using it.

…#7988) Drive the loading state through NcSelect's own `search`-event callback instead of the reactive `loadingGroups`/`:disabled` binding. Toggling `:disabled` per keystroke disabled the focused text input and dropped focus on every character; routing the spinner through vue-select's `loading` callback keeps the input enabled throughout the search. `loadingGroups` now only guards the initial onMounted load. Adopts the simpler approach proposed by the issue reporter. Signed-off-by: Ethan Hawkes <ethanhawkes-gif@users.noreply.github.com> Co-authored-by: Spitfireap <45575529+Spitfireap@users.noreply.github.com>

Spitfireap · 2026-08-17T10:14:09Z

Wonderful, tested on my instance and it works. Haven't checked/tested the test suite changes though.

Spitfireap · 2026-08-17T11:21:21Z

I'm questioning myself about about this line.
Since we export searchGroups perhaps it would be better that this function triggers the loadingGroups variable. We could then create a function that actually fetches groups via the API like searchGroupApi and make both searchGroup and searchGroupEvent use it instead.

That way if any parent component triggers searchGroup the user will be informed. searchGroupApi could be private in that case. Wdyt @vitormattos

Extract the raw group fetch into a private `searchGroupApi` and let the exposed `searchGroup` wrap it, driving the reactive `loadingGroups`/`:disabled` binding so any parent component that calls the exposed function gets a visible loading state. `searchGroupEvent` (the NcSelect `@search` handler) keeps driving only vue-select's own `loading()` callback via `searchGroupApi`, so the text input stays enabled and never loses focus while typing (LibreSign#7988). The initial onMounted load now relies on `searchGroup`'s own toggling. Adopts the structure proposed by the issue reporter. Signed-off-by: Ethan Hawkes <ethanhawkes-gif@users.noreply.github.com> Co-authored-by: Spitfireap <45575529+Spitfireap@users.noreply.github.com>

ethanhawkes-gif · 2026-08-17T11:58:24Z

Good call — that's cleaner than what was there, and it fixes a real inconsistency: the exposed searchGroup previously gave no loading feedback at all. Pushed as afb65ab, exactly along the lines you described:

searchGroupApi is now the private fetch. searchGroup wraps it and drives loadingGroups/:disabled, so any parent component that calls the exposed function gets a visible loading state.
searchGroupEvent (the @search handler) still drives only NcSelect's own loading() callback — via searchGroupApi — so typing never disables or blurs the input (AllowedGroups does not retrieve the searched group #7988).
onMounted just awaits searchGroup('') now, since it owns its own loading toggle.

Full front-end suite (3042 tests) and eslint pass locally. The structure is your design, so the commit carries your co-author trailer. Happy to leave the final architectural call to the maintainer.

And yes to your earlier question — there's AI assistance behind this account. The code, tests, and review are real and I stand behind them.

Spitfireap

Some tests need to be updated

Spitfireap · 2026-08-17T12:21:40Z

 					NcSelect: {
 						name: 'NcSelect',
 						props: ['modelValue', 'ariaLabelCombobox'],
 						emits: ['update:modelValue', 'search-change'],


I think that this should be updated reflecting the change of search-change to search

Spitfireap · 2026-08-17T12:21:59Z

 					NcSelect: {
 						name: 'NcSelect',
 						props: ['modelValue'],
 						emits: ['update:modelValue', 'search-change'],


Spitfireap · 2026-08-17T12:22:16Z

 					NcSelect: {
 						name: 'NcSelect',
 						props: ['modelValue'],
 						emits: ['update:modelValue', 'search-change'],


Spitfireap · 2026-08-17T12:22:28Z

 					NcSelect: {
 						name: 'NcSelect',
 						props: ['modelValue'],
 						emits: ['update:modelValue', 'search-change'],


vitormattos

Thanks for working on this.

I confirmed the original issue and the event change is correct. The regression test also demonstrates that the previous search-change listener did not trigger the backend search.

Before approving this, I would like a few things cleaned up.

Please update the remaining NcSelect test stubs that still declare search-change, so the tests reflect the real component API.

Also, please update the PR description to match the final implementation. It currently describes @search="searchGroup", while the final code uses searchGroupEvent and separates the API call into searchGroupApi.

Since this PR has changed significantly during the review, please also manually validate the final version and describe, please explain the design decisions, why we now have searchGroupApi, searchGroup, and searchGroupEvent, and why the search event should use the loading callback instead of loadingGroups.

AI assistance itself is not a problem. But contributors are still responsible for understanding, testing, and being able to maintain the code they submit. I want to make sure the final code is something you have actually reviewed and understood, rather than just the result of applying suggestions.

Once these points are addressed, I can review the final version again.

Spitfireap · 2026-08-18T20:34:15Z

Thanks for taking a look at it. Not an AI myself and I cannot speak for the tests but as for the implementatiom changes it looks good to me.
I let @ethanhawkes-gif answer your comment

github-project-automation Bot added this to Roadmap Aug 17, 2026

github-project-automation Bot moved this to 0. Backlog in Roadmap Aug 17, 2026

ethanhawkes-gif mentioned this pull request Aug 17, 2026

AllowedGroups does not retrieve the searched group #7988

Open

ethanhawkes-gif force-pushed the fix/allowed-groups-search-event branch from 47b0f23 to 74c9678 Compare August 17, 2026 10:04

Spitfireap suggested changes Aug 17, 2026

View reviewed changes

github-project-automation Bot moved this from 0. Backlog to 1. to do in Roadmap Aug 17, 2026

vitormattos reviewed Aug 18, 2026

View reviewed changes

Uh oh!

Uh oh!

Conversation

ethanhawkes-gif commented Aug 17, 2026

Summary

Root cause

Fix

Tests

Uh oh!

Spitfireap commented Aug 17, 2026

Uh oh!

ethanhawkes-gif commented Aug 17, 2026

Uh oh!

Spitfireap commented Aug 17, 2026

Uh oh!

Spitfireap commented Aug 17, 2026

Uh oh!

Spitfireap commented Aug 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ethanhawkes-gif commented Aug 17, 2026

Uh oh!

Spitfireap left a comment

Choose a reason for hiding this comment

Uh oh!

Spitfireap Aug 17, 2026

Choose a reason for hiding this comment

Uh oh!

Spitfireap Aug 17, 2026

Choose a reason for hiding this comment

Uh oh!

Spitfireap Aug 17, 2026

Choose a reason for hiding this comment

Uh oh!

Spitfireap Aug 17, 2026

Choose a reason for hiding this comment

Uh oh!

vitormattos left a comment

Choose a reason for hiding this comment

Uh oh!

Spitfireap commented Aug 18, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Spitfireap commented Aug 17, 2026 •

edited

Loading