feat: add filter-based mass selection via search modal#12901
Open
RobinAngele wants to merge 25 commits intonextcloud:mainfrom
Open
feat: add filter-based mass selection via search modal#12901RobinAngele wants to merge 25 commits intonextcloud:mainfrom
RobinAngele wants to merge 25 commits intonextcloud:mainfrom
Conversation
…ctions - Add missing ImportantIcon import and component registration in EnvelopeList so the 'Mark as important' icon renders during bulk selection - Fix favorite/unfavorite bulk action logic: rename methods to favoriteAll/unfavoriteAll and use explicit favFlag values (true/false) instead of inverted computed checks that failed when all selected messages shared the same state AI-assisted: Cline (Claude) Signed-off-by: RobinAngele <robin@robin4consulting.com>
Add a 'Select all X messages' checkbox above the envelope list using NcCheckboxRadioSwitch from @nextcloud/vue, allowing users to select all visible messages at once. Lift envelope selection state from individual EnvelopeList instances up to the Mailbox parent component. This enables: - Cross-group shift-click range selection via flat envelope indexing - Consistent selection state across grouped envelope lists - Global select-all / unselect-all from the parent level Also pass the selection array as a prop to EnvelopeList and add proper event handling for update:selection and select-range. Closes: nextcloud#4285 Refs: nextcloud#7880, nextcloud#6070, nextcloud#7276 AI-assisted: Cline (Claude) Signed-off-by: RobinAngele <robin@robin4consulting.com>
Add a 'Select all matching' button to the search parameters dialog and enable selecting all messages matching a filter across all pages. Search modal: - New 'Select all matching' button in SearchMessages.vue dialog - Emits 'select-all-matching' event via the MailboxThread bus Mass loading: - Mailbox.vue onBusSelectAllMatching forces a fresh load, then iterates loadMore() until all pages are fetched - Spinner + 'Selecting messages…' shown during loading - Checkbox disabled while loading Context-aware labels: - 'Select N loaded messages' when more pages exist - 'Select N matching messages' when a filter is active - 'Select all N messages' when all pages are loaded - Hint row: 'Scroll down to include more messages, use filter to refine, or click an avatar circle to select one at a time' Depends on: nextcloud#12900 (select-all checkbox feature) Closes: nextcloud#4285, nextcloud#7880 Refs: nextcloud#6070, nextcloud#7276, nextcloud#11526 AI-assisted: Cline (Claude) Signed-off-by: RobinAngele <robin@robin4consulting.com>
RobinAngele
requested review from
ChristophWurst,
GretaD and
kesselb
as code owners
This was referenced May 7, 2026
Closed
added 3 commits
May 8, 2026 08:01
Use selectMode (any selection) instead of allSelected (must match all visible) for the checkbox checked state. This way the checkbox stays checked when the user manually deselects individual messages via avatar circles after using Select All. AI-assisted: Cline (Claude) Signed-off-by: RobinAngele <robin@robin4consulting.com>
…s active When a filter is already applied, the hint now shows: 'Scroll down to include more messages or click an avatar circle to select one at a time' — removing the redundant filter advice. AI-assisted: Cline (Claude) Signed-off-by: RobinAngele <robin@robin4consulting.com>
When in selectMode, the checkbox label now shows '{N} selected'
with the real count instead of the loaded count. This way when
the user deselects individual messages, the number updates
immediately from 20 to 19, etc.
AI-assisted: Cline (Claude)
Signed-off-by: RobinAngele <robin@robin4consulting.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR lifts selection state to Mailbox, adds a select-all UI and "select all matching" flow, converts EnvelopeList to a prop-driven selection model with range-selection delegated to Mailbox, and adds SearchMessages dialog button plus MailboxThread wiring to trigger cross-page selection. ChangesSelect All Messages Feature
🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/Mailbox.vue (1)
317-335:⚠️ Potential issue | 🟠 Major | ⚡ Quick winReset pagination and bulk-selection state when the source list changes.
These watchers clear
selection, but they leaveendReached/expandedbehind and only partially reset the select-all-matching flags. If the previous view had already reached the end, the next mailbox/filter/sort starts in a stale “fully loaded” state, which can hide further pages and prevent the new result set from using the intended bulk-select flow.Suggested reset
mailbox() { this.selection = [] + this.selectAllMatching = false + this.loadingAllMatching = false + this.endReached = false + this.expanded = false this.loadEnvelopes() .then(() => { logger.debug(`syncing mailbox ${this.mailbox.databaseId} (${this.query}) after folder change`) this.sync(false) }) }, searchQuery() { this.selection = [] this.selectAllMatching = false this.loadingAllMatching = false + this.endReached = false + this.expanded = false this.loadEnvelopes() }, sortOrder() { this.selection = [] + this.selectAllMatching = false + this.loadingAllMatching = false + this.endReached = false + this.expanded = false this.loadEnvelopes() },🤖 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 `@src/components/Mailbox.vue` around lines 317 - 335, The watchers mailbox(), searchQuery(), and sortOrder() currently clear selection but leave pagination and bulk-selection state stale; update each watcher to also reset endReached (set to false), clear expanded state (reset the expanded map/array), and set selectAllMatching and loadingAllMatching to false before calling loadEnvelopes() (and before sync in mailbox()), so the new mailbox/filter/sort starts with fresh pagination and bulk-selection state; reference the mailbox(), searchQuery(), and sortOrder() watcher handlers and the component properties endReached, expanded, selectAllMatching, loadingAllMatching, selection, loadEnvelopes(), and sync().
🤖 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 `@src/components/MailboxThread.vue`:
- Around line 607-609: The onSelectAllMatching handler currently emits a generic
'select-all-matching' on the shared mitt bus causing every Mailbox subscriber to
react; change it to emit a scoped payload (e.g., include mailboxId or query) or
target the specific Mailbox directly. Update the onSelectAllMatching method to
call this.bus.emit('select-all-matching', { mailboxId: this.mailboxId, query:
this.currentQuery }) (or invoke a direct method on the intended Mailbox
instance) and update Mailbox subscribers to check the payload (e.g., mailboxId
or query) before starting load/select so only the intended mailbox responds.
Ensure the event name remains 'select-all-matching' but listeners in Mailbox use
the identifying field to ignore unrelated events.
In `@src/components/SearchMessages.vue`:
- Around line 578-584: The selectAllMatching() method currently calls
sendQueryEvent() and emits 'select-all-matching' inside a single
this.$nextTick(), which can let Mailbox receive a stale
MailboxThread.searchQuery prop; change the flow in selectAllMatching to call
this.sendQueryEvent() inside the first this.$nextTick(), then call a second
this.$nextTick() before emitting this.$emit('select-all-matching') so
Mailbox.onBusSelectAllMatching() receives the updated prop value and avoids
corrupting the cache key logic.
---
Outside diff comments:
In `@src/components/Mailbox.vue`:
- Around line 317-335: The watchers mailbox(), searchQuery(), and sortOrder()
currently clear selection but leave pagination and bulk-selection state stale;
update each watcher to also reset endReached (set to false), clear expanded
state (reset the expanded map/array), and set selectAllMatching and
loadingAllMatching to false before calling loadEnvelopes() (and before sync in
mailbox()), so the new mailbox/filter/sort starts with fresh pagination and
bulk-selection state; reference the mailbox(), searchQuery(), and sortOrder()
watcher handlers and the component properties endReached, expanded,
selectAllMatching, loadingAllMatching, selection, loadEnvelopes(), and sync().
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 495a2697-7608-4c97-89cd-216e17ceac3f
📒 Files selected for processing (4)
src/components/EnvelopeList.vuesrc/components/Mailbox.vuesrc/components/MailboxThread.vuesrc/components/SearchMessages.vue
Comment on lines
+607
to
+609
| onSelectAllMatching() { | ||
| // Signal the Mailbox component to select all after loading finishes | ||
| this.bus.emit('select-all-matching') |
There was a problem hiding this comment.
Scope select-all-matching to one mailbox instance.
This emits on the shared mitt bus that every Mailbox in the view subscribes to. In views with favorites / priority sections, clicking the modal action will make each section start loading and selecting independently instead of only targeting the main result set. Send a payload that identifies the intended mailbox/query, or wire this action directly to the target Mailbox.
🤖 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 `@src/components/MailboxThread.vue` around lines 607 - 609, The
onSelectAllMatching handler currently emits a generic 'select-all-matching' on
the shared mitt bus causing every Mailbox subscriber to react; change it to emit
a scoped payload (e.g., include mailboxId or query) or target the specific
Mailbox directly. Update the onSelectAllMatching method to call
this.bus.emit('select-all-matching', { mailboxId: this.mailboxId, query:
this.currentQuery }) (or invoke a direct method on the intended Mailbox
instance) and update Mailbox subscribers to check the payload (e.g., mailboxId
or query) before starting load/select so only the intended mailbox responds.
Ensure the event name remains 'select-all-matching' but listeners in Mailbox use
the identifying field to ignore unrelated events.
added 2 commits
May 8, 2026 09:30
Added _localToggleInProgress guard that prevents the selection watcher from overwriting flags.selected during a local click toggle. This fixes a bug where clicking one avatar to deselect would sometimes remove multiple messages at once. AI-assisted: Cline (Claude) Signed-off-by: RobinAngele <robin@robin4consulting.com>
When loadingAllMatching is true, selectAllLabel now returns 'Selecting messages…' instead of the contextual label (matching/loaded/all). This prevents showing stale filter labels while loading is in progress. AI-assisted: Cline (Claude) Signed-off-by: RobinAngele <robin@robin4consulting.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/components/Mailbox.vue (2)
830-846: ⚖️ Poor tradeoffConsider adding a cancellation mechanism for long-running operations.
The
while (!this.endReached)loop could run for a very long time on large mailboxes, especially over slow connections. If the user changes the filter, navigates away, or the component unmounts during loading, the loop continues until completion.Consider:
- Adding an abort flag checked in the loop
- Implementing a page limit (e.g., max 50 pages)
- Checking component mount state before state updates
♻️ Example cancellation pattern
async selectAllMatchingAction() { this.loadingAllMatching = true this.selectAllMatching = true + this._selectAllAborted = false try { // Load remaining pages until all envelopes are fetched - while (!this.endReached) { + while (!this.endReached && !this._selectAllAborted) { await this.loadMore() } + if (this._selectAllAborted) { + return + } // Now select all loaded envelopes this.selection = this.flatEnvelopeList.map((e) => e.databaseId) } catch (error) { logger.error('Failed to load all matching envelopes', { error }) } finally { this.loadingAllMatching = false } },Then set
this._selectAllAborted = trueinunselectAll()and watchers that reset state.🤖 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 `@src/components/Mailbox.vue` around lines 830 - 846, The selectAllMatchingAction can run indefinitely; add a cancellable pattern: introduce a boolean flag (e.g. this._selectAllAborted) and a maxPage cap (e.g. const MAX_SELECT_PAGES = 50) and modify selectAllMatchingAction to check both the abort flag and page cap inside the while (!this.endReached) loop before each await this.loadMore() and break if triggered; ensure you check the abort flag before assigning this.selection or calling this.flatEnvelopeList, and clear loadingAllMatching on abort; set this._selectAllAborted = true from unselectAll(), relevant filter-change watchers, and the component beforeDestroy/unmounted hook so the in-flight loop exits and avoids updating state after unmount.
36-48: 💤 Low valueDead code: loading icon inside the banner will never render.
The banner's
v-ifcondition includes!selectAllMatching, butloadingAllMatchingandselectAllMatchingare always set together (both inselectAllMatchingActionandonBusSelectAllMatching). WhenloadingAllMatchingis true,selectAllMatchingis also true, so the banner won't render and theNcLoadingIconon line 39 is unreachable.Consider removing the loading icon from the banner or adjusting the banner's visibility condition if loading state should be shown here.
♻️ Suggested fix
<div v-if="allSelected && !selectAllMatching && flatEnvelopeList.length < totalEnvelopeCount" class="select-all-banner"> - <NcLoadingIcon v-if="loadingAllMatching" :size="16" /> <span>{{ n('mail',🤖 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 `@src/components/Mailbox.vue` around lines 36 - 48, The banner's v-if (allSelected && !selectAllMatching && flatEnvelopeList.length < totalEnvelopeCount) prevents loadingAllMatching from ever being visible because selectAllMatching is set alongside loadingAllMatching in selectAllMatchingAction and onBusSelectAllMatching; either remove the unreachable NcLoadingIcon or change the banner condition to allow the loading state — e.g. update the v-if to include loadingAllMatching (allSelected && (loadingAllMatching || !selectAllMatching) && flatEnvelopeList.length < totalEnvelopeCount) so NcLoadingIcon can render while select-all is in progress, or simply delete the NcLoadingIcon element if you prefer not to show loading there.
🤖 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 `@src/components/Mailbox.vue`:
- Around line 21-28: The checkbox is listening to the wrong event: change the
event handler on the NcCheckboxRadioSwitch from `@update`:checked to
`@update`:model-value so toggles actually invoke your handlers; update the
component invocation that references selectMode and the methods selectAll() /
unselectAll() to use `@update`:model-value="selectMode ? unselectAll() :
selectAll()" so the v-model update triggers those functions.
---
Nitpick comments:
In `@src/components/Mailbox.vue`:
- Around line 830-846: The selectAllMatchingAction can run indefinitely; add a
cancellable pattern: introduce a boolean flag (e.g. this._selectAllAborted) and
a maxPage cap (e.g. const MAX_SELECT_PAGES = 50) and modify
selectAllMatchingAction to check both the abort flag and page cap inside the
while (!this.endReached) loop before each await this.loadMore() and break if
triggered; ensure you check the abort flag before assigning this.selection or
calling this.flatEnvelopeList, and clear loadingAllMatching on abort; set
this._selectAllAborted = true from unselectAll(), relevant filter-change
watchers, and the component beforeDestroy/unmounted hook so the in-flight loop
exits and avoids updating state after unmount.
- Around line 36-48: The banner's v-if (allSelected && !selectAllMatching &&
flatEnvelopeList.length < totalEnvelopeCount) prevents loadingAllMatching from
ever being visible because selectAllMatching is set alongside loadingAllMatching
in selectAllMatchingAction and onBusSelectAllMatching; either remove the
unreachable NcLoadingIcon or change the banner condition to allow the loading
state — e.g. update the v-if to include loadingAllMatching (allSelected &&
(loadingAllMatching || !selectAllMatching) && flatEnvelopeList.length <
totalEnvelopeCount) so NcLoadingIcon can render while select-all is in progress,
or simply delete the NcLoadingIcon element if you prefer not to show loading
there.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: e480ccd4-a6cf-4ccc-9a40-65b3f878b3f0
📒 Files selected for processing (2)
src/components/EnvelopeList.vuesrc/components/Mailbox.vue
Comment on lines
+21
to
+28
| <NcCheckboxRadioSwitch | ||
| :model-value="selectMode" | ||
| :disabled="loadingAllMatching" | ||
| type="checkbox" | ||
| class="select-all-bar" | ||
| @update:checked="selectMode ? unselectAll() : selectAll()"> | ||
| {{ selectAllLabel }} | ||
| </NcCheckboxRadioSwitch> |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
NcCheckboxRadioSwitch @nextcloud/vue event name modelValue
💡 Result:
In @nextcloud/vue, NcCheckboxRadioSwitch uses Vue 3 v-model with the prop modelValue and emits update:modelValue (not a custom event like change/input). This is the standard Vue component v-model contract: modelValue prop + update:modelValue event [1]. Nextcloud specifically renamed the old “checked” prop to modelValue so you can use it as v-model (e.g., v-model="isChecked") for NcCheckboxRadioSwitch (and related components) [2]. So the event name you’re looking for is: update:modelValue If you previously used v-model:checked, that prop/event pairing was replaced by v-model/modelValue + update:modelValue [2].
Citations:
- 1: https://vuejs.org/guide/components/v-model
- 2: https://github.com/nextcloud-libraries/nextcloud-vue/pull/4994/files
🏁 Script executed:
# Find the import statement for NcCheckboxRadioSwitch in Mailbox.vue
rg -n "NcCheckboxRadioSwitch|import" src/components/Mailbox.vue | head -20Repository: nextcloud/mail
Length of output: 1066
Change @update:checked to @update:model-value for correct event binding.
The NcCheckboxRadioSwitch component from @nextcloud/vue emits @update:model-value (the Vue 3 v-model contract with the modelValue prop), not @update:checked. The current binding will not trigger the selectAll() or unselectAll() methods when the checkbox is toggled.
Update line 25 from:
`@update`:checked="selectMode ? unselectAll() : selectAll()">
to:
`@update`:model-value="selectMode ? unselectAll() : selectAll()">
🤖 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 `@src/components/Mailbox.vue` around lines 21 - 28, The checkbox is listening
to the wrong event: change the event handler on the NcCheckboxRadioSwitch from
`@update`:checked to `@update`:model-value so toggles actually invoke your handlers;
update the component invocation that references selectMode and the methods
selectAll() / unselectAll() to use `@update`:model-value="selectMode ?
unselectAll() : selectAll()" so the v-model update triggers those functions.
Vue 2 batches watchers at end of tick. The previous synchronous reset meant the watcher would fire after _localToggleInProgress was already false, re-introducing the race condition. Now reset via $nextTick so the guard remains active during the watcher's flush. AI-assisted: Cline (Claude) Signed-off-by: RobinAngele <robin@robin4consulting.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/components/EnvelopeList.vue (1)
393-404: 💤 Low valueConsider removing redundant cleanup code.
The new cleanup logic (lines 395-399) correctly removes vanished envelope IDs from selection and emits the cleaned array. The older
differenceWithcleanup (lines 400-403) setsflags.selected = falseon envelopes that are no longer insortedEnvelops, which has no practical effect since those envelopes are already removed from the rendered list. This code could be removed for clarity.♻️ Suggested cleanup
sortedEnvelops(newVal, oldVal) { // Unselect vanished envelopes by emitting cleaned selection const newIds = new Set(newVal.map((env) => env.databaseId)) const cleanedSelection = this.selection.filter((id) => newIds.has(id)) if (cleanedSelection.length !== this.selection.length) { this.$emit('update:selection', cleanedSelection, this.envelopes) } - differenceWith((a, b) => a.databaseId === b.databaseId, oldVal, newVal) - .forEach((env) => { - env.flags.selected = false - }) },🤖 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 `@src/components/EnvelopeList.vue` around lines 393 - 404, Remove the redundant cleanup in the sortedEnvelops watcher: the initial block that builds newIds, computes cleanedSelection from this.selection and emits update:selection already cleans vanished IDs, so delete the subsequent differenceWith(...).forEach(env => env.flags.selected = false) code path (referenced as differenceWith in the sortedEnvelops watcher which manipulates env.flags.selected) to avoid dead/unused state changes while keeping the selection cleanup and this.$emit('update:selection', cleanedSelection, this.envelopes) intact.
🤖 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.
Nitpick comments:
In `@src/components/EnvelopeList.vue`:
- Around line 393-404: Remove the redundant cleanup in the sortedEnvelops
watcher: the initial block that builds newIds, computes cleanedSelection from
this.selection and emits update:selection already cleans vanished IDs, so delete
the subsequent differenceWith(...).forEach(env => env.flags.selected = false)
code path (referenced as differenceWith in the sortedEnvelops watcher which
manipulates env.flags.selected) to avoid dead/unused state changes while keeping
the selection cleanup and this.$emit('update:selection', cleanedSelection,
this.envelopes) intact.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 32a554d8-9c2d-4cd5-bfdc-7a6234f76eed
📒 Files selected for processing (1)
src/components/EnvelopeList.vue
Added event.shiftKey check in onSelectMultiple to prevent range selection from firing on plain clicks. Also passes $event from template to the handler. AI-assisted: Cline (Claude) Signed-off-by: RobinAngele <robin@robin4consulting.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/Envelope.vue (1)
1-4:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUpdate SPDX header to the required repository format.
Line 2 and Line 3 do not match the mandatory SPDX strings for Vue files in this repository (year/text and identifier key).
🔧 Proposed fix
<!-- - - SPDX-FileCopyrightText: 2018 Nextcloud GmbH and Nextcloud contributors - - SPDX-License-Identifier: AGPL-3.0-or-later + - SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors + - SPDX-Identifier: AGPL-3.0-or-later -->As per coding guidelines "
{lib,src,tests}/**/*.{php,js,vue}: Every file must include an SPDX license header" and "SPDX license header format must be:SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributorsandSPDX-Identifier: AGPL-3.0-or-later".🤖 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 `@src/components/Envelope.vue` around lines 1 - 4, Update the SPDX header in Envelope.vue to the repository-mandated format by replacing the existing two header lines with the exact required strings: "SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors" and "SPDX-Identifier: AGPL-3.0-or-later"; locate the existing header comment at the top of src/components/Envelope.vue and swap those two lines so the file matches the project's SPDX header rules.
🤖 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.
Outside diff comments:
In `@src/components/Envelope.vue`:
- Around line 1-4: Update the SPDX header in Envelope.vue to the
repository-mandated format by replacing the existing two header lines with the
exact required strings: "SPDX-FileCopyrightText: 2026 Nextcloud GmbH and
Nextcloud contributors" and "SPDX-Identifier: AGPL-3.0-or-later"; locate the
existing header comment at the top of src/components/Envelope.vue and swap those
two lines so the file matches the project's SPDX header rules.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: f1de2789-e604-4606-890e-aa1cb989082a
📒 Files selected for processing (1)
src/components/Envelope.vue
…parison Changed hasFilter check from !== 'match:allof' to .length > 12. This handles cases where closeSearchModal emits a stale query with residual whitespace or parameters. The default match:allof query is exactly 12 chars; anything longer has a real filter. AI-assisted: Cline (Claude) Signed-off-by: RobinAngele <robin@robin4consulting.com>
RobinAngele
force-pushed
the
feat/filter-mass-select
branch
from
d96ac21 to
c5a8cfd
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/Mailbox.vue (1)
323-341:⚠️ Potential issue | 🟠 Major | ⚡ Quick winReset pagination and mass-selection flags when the result set changes.
These watchers clear
selection, but they leaveendReached/expandedbehind, andmailbox/sortOrderalso leave the bulk-selection flags behind. After fully loading one folder or filter, the next one can start withendReached = true, which hides pagination and makes the new select-all flow stop early.Suggested fix
watch: { mailbox() { - this.selection = [] + this.unselectAll() + this.endReached = false + this.expanded = false this.loadEnvelopes() .then(() => { logger.debug(`syncing mailbox ${this.mailbox.databaseId} (${this.query}) after folder change`) this.sync(false) }) }, searchQuery() { - this.selection = [] - this.selectAllMatching = false - this.loadingAllMatching = false + this.unselectAll() + this.endReached = false + this.expanded = false this.loadEnvelopes() }, sortOrder() { - this.selection = [] + this.unselectAll() + this.endReached = false + this.expanded = false this.loadEnvelopes() }, },🤖 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 `@src/components/Mailbox.vue` around lines 323 - 341, The mailbox(), searchQuery(), and sortOrder() watchers currently only clear selection but must also reset pagination and mass-selection flags to avoid carrying state into the next result set; update each watcher (mailbox(), searchQuery(), sortOrder()) to set endReached = false and expanded = false and also clear selectAllMatching = false and loadingAllMatching = false before calling loadEnvelopes()/sync(), ensuring any pagination cursor or offset state used by loadEnvelopes() is effectively reset for the new folder/filter.
🤖 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 `@src/components/Mailbox.vue`:
- Around line 37-47: The banner/button currently uses totalEnvelopeCount as if
it were the full match total even when endReached is false; update the template
logic around the banner (the v-if using allSelected, selectAllMatching and
flatEnvelopeList.length < totalEnvelopeCount) and the copy/CTA (the span,
NcButton and their use of totalEnvelopeCount) so that exact "all matching"
counts are only shown when endReached === true—otherwise show a non-exact CTA
like "Select all visible messages" or "Select all matching messages" (no exact
number) and, if desired, show a progressive "Select all {visible} on this page"
with a secondary loader state (loadingAllMatching) that then resolves to the
exact count when endReached becomes true; apply the same change to the second
identical block that uses the same variables and selectAllMatchingAction.
- Around line 831-846: The selectAllMatchingAction loop can spin indefinitely
when loadMore() throws because endReached never flips; update
selectAllMatchingAction so that if loadMore() rejects you break the while loop
(or set endReached=true) before rethrowing or logging. Specifically, inside
selectAllMatchingAction around the while (!this.endReached) { await
this.loadMore() } call handle errors from loadMore() (or wrap each await
this.loadMore() in its own try/catch) and on error log via logger.error and then
break the loop so the method can proceed to set this.selection from
this.flatEnvelopeList and finish (preserve the existing finally that clears
this.loadingAllMatching and leave this.selectAllMatching true/false behavior
unchanged).
---
Outside diff comments:
In `@src/components/Mailbox.vue`:
- Around line 323-341: The mailbox(), searchQuery(), and sortOrder() watchers
currently only clear selection but must also reset pagination and mass-selection
flags to avoid carrying state into the next result set; update each watcher
(mailbox(), searchQuery(), sortOrder()) to set endReached = false and expanded =
false and also clear selectAllMatching = false and loadingAllMatching = false
before calling loadEnvelopes()/sync(), ensuring any pagination cursor or offset
state used by loadEnvelopes() is effectively reset for the new folder/filter.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 7a525e38-36d4-4e8e-8869-8d36f335f18c
📒 Files selected for processing (1)
src/components/Mailbox.vue
Comment on lines
+831
to
+846
| async selectAllMatchingAction() { | ||
| this.loadingAllMatching = true | ||
| this.selectAllMatching = true | ||
|
|
||
| try { | ||
| // Load remaining pages until all envelopes are fetched | ||
| while (!this.endReached) { | ||
| await this.loadMore() | ||
| } | ||
| // Now select all loaded envelopes | ||
| this.selection = this.flatEnvelopeList.map((e) => e.databaseId) | ||
| } catch (error) { | ||
| logger.error('Failed to load all matching envelopes', { error }) | ||
| } finally { | ||
| this.loadingAllMatching = false | ||
| } |
There was a problem hiding this comment.
Break the mass-selection loop when loadMore() fails.
This loop assumes loadMore() will always make progress or flip endReached. It does neither on fetch errors right now, so one failing page request turns “Select all matching” into an endless retry loop with a stuck spinner.
Suggested fix
async loadMore() {
if (!this.expanded && this.envelopesToShow.length < this.envelopes.length) {
logger.debug('expanding envelope list')
this.expanded = true
- return
+ return true
}
logger.debug('fetching next envelope page')
this.loadingMore = true
try {
const envelopes = await this.mainStore.fetchNextEnvelopePage({
mailboxId: this.mailbox.databaseId,
query: this.searchQuery,
})
if (envelopes.length === 0) {
logger.info('envelope list end reached')
this.endReached = true
}
+ return true
} catch (error) {
logger.error('could not fetch next envelope page', { error })
+ return false
} finally {
this.loadingMore = false
}
}
async selectAllMatchingAction() {
this.loadingAllMatching = true
this.selectAllMatching = true
try {
while (!this.endReached) {
- await this.loadMore()
+ const ok = await this.loadMore()
+ if (!ok) {
+ throw new Error('Could not load remaining envelopes')
+ }
}
this.selection = this.flatEnvelopeList.map((e) => e.databaseId)
} catch (error) {
logger.error('Failed to load all matching envelopes', { error })
} finally {
this.loadingAllMatching = false
}
}🤖 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 `@src/components/Mailbox.vue` around lines 831 - 846, The
selectAllMatchingAction loop can spin indefinitely when loadMore() throws
because endReached never flips; update selectAllMatchingAction so that if
loadMore() rejects you break the while loop (or set endReached=true) before
rethrowing or logging. Specifically, inside selectAllMatchingAction around the
while (!this.endReached) { await this.loadMore() } call handle errors from
loadMore() (or wrap each await this.loadMore() in its own try/catch) and on
error log via logger.error and then break the loop so the method can proceed to
set this.selection from this.flatEnvelopeList and finish (preserve the existing
finally that clears this.loadingAllMatching and leave this.selectAllMatching
true/false behavior unchanged).
Member
|
Sorry if the review rabbit caused any confusion. We enabled the integration but the config to turn auto reviews off was missing. |
added 6 commits
May 8, 2026 10:43
After a filter loads all pages, endReached stays true. When the filter is cleared, loadEnvelopes() is called but doesn't reset endReached, causing the label to skip the 'loaded' state and show the default 'Select all N messages' instead. Now the searchQuery watcher resets endReached before reloading. AI-assisted: Cline (Claude) Signed-off-by: RobinAngele <robin@robin4consulting.com>
The previous .length > 12 check could fail if the default match:allof query had extra whitespace or encoding artifacts. Now uses /^match:allof\s*$/ regex for robust detection. AI-assisted: Cline (Claude) Signed-off-by: RobinAngele <robin@robin4consulting.com>
mentionsMe is a boolean. After resetFilter(), it's false. filterData returned the raw boolean, and searchQuery builder added 'mentions:false' to the query because it only skipped empty strings and null, not false. Now returns empty string when mentionsMe is false. AI-assisted: Cline (Claude) Signed-off-by: RobinAngele <robin@robin4consulting.com>
Previously only loadingAllMatching prevented stale labels. Now loadingEnvelopes (normal filter/mailbox change) also shows 'Loading…' and disables the checkbox, preventing stale counts from appearing during async loads. AI-assisted: Cline (Claude) Signed-off-by: RobinAngele <robin@robin4consulting.com>
The sortedEnvelops watcher could fire during a local toggle and clear flags.selected on active envelopes via differenceWith. Added _localToggleInProgress guard to prevent this. AI-assisted: Cline (Claude) Signed-off-by: RobinAngele <robin@robin4consulting.com>
Normal envelope loading now only changes the label to 'Loading…' without disabling the checkbox or showing the spinner row. This prevents the stuck spinner when rapid filter changes cause overlapping loadEnvelopes calls. AI-assisted: Cline (Claude) Signed-off-by: RobinAngele <robin@robin4consulting.com>
added 8 commits
May 8, 2026 17:18
…Event Removed $nextTick from selectAllMatching() so events fire immediately. Added _closingProgrammatically guard to closeSearchModal to prevent it from emitting a duplicate search-changed when the dialog closes programmatically via selectAllMatching. AI-assisted: Cline (Claude) Signed-off-by: RobinAngele <robin@robin4consulting.com>
NcDialog's @closing fires asynchronously after moreSearchActions changes. By resetting the flag in $nextTick, it stays true when closeSearchModal runs, preventing the duplicate sendQueryEvent. AI-assisted: Cline (Claude) Signed-off-by: RobinAngele <robin@robin4consulting.com>
…hods - hasFilter as computed (single regex, reused by label + hint) - closeSearchModal no longer emits (only explicit button clicks) Removes _closingProgrammatically guard entirely - Merged selectAllMatchingAction into onBusSelectAllMatching AI-assisted: Cline (Claude) Signed-off-by: RobinAngele <robin@robin4consulting.com>
search-changed updates the searchQuery prop synchronously but Vue batches prop updates. select-all-matching was firing before the batch flushed, causing onBusSelectAllMatching to use the old query. Now wrapped in $nextTick so the new prop value is available. AI-assisted: Cline (Claude) Signed-off-by: RobinAngele <robin@robin4consulting.com>
Instead of reading this.searchQuery prop (which might be stale due to Vue batching), the query string is now passed through the entire event chain: SearchMessages -> MailboxThread bus -> Mailbox. This guarantees the correct query on first click. AI-assisted: Cline (Claude) Signed-off-by: RobinAngele <robin@robin4consulting.com>
search-changed watcher was resetting loadingAllMatching and selectAllMatching after onBusSelectAllMatching set them, causing the spinner and mass selection to abort. Now the watcher checks this.selectAllMatching and skips reset when the bus handler is managing the state. AI-assisted: Cline (Claude) Signed-off-by: RobinAngele <robin@robin4consulting.com>
…tAllMatching selectAllMatching stays true after mass selection completes, blocking all future watcher calls. _busHandlerActive is only true during the bus handler's execution and cleared in finally. AI-assisted: Cline (Claude) Signed-off-by: RobinAngele <robin@robin4consulting.com>
Removing sendQueryEvent from closeSearchModal broke the normal Search button - filters were never applied. Restored with the _busHandlerActive guard still protecting against watcher overwrite. AI-assisted: Cline (Claude) Signed-off-by: RobinAngele <robin@robin4consulting.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Depends on: #12900
Closes #4285
Closes #7880
Refs #6070
Refs #7276
Summary
Adds a "Select all matching" button to the search dialog. Loads all messages matching the current filter across every page and selects them for bulk actions — completing the feature requested in #4285 and #7880.
Built on #12900 which centralized selection state in
Mailbox.Architecture
Two events fire on the same click:
search-changedupdates thesearchQueryprop, andselect-all-matchingtriggers mass loading. Three mechanisms handle the resulting race conditions:1. Query passed as parameter. The filter string is captured at click time and handed through every step —
Mailbox.onBusSelectAllMatching(query)callsloadEnvelopes(query)directly. The prop is never read by the bus handler, so Vue's batching cannot deliver a stale value.2.
_busHandlerActiveguard in the watcher. Closing the dialog callscloseSearchModal(), which also emitssearch-changed. Without the guard, the watcher would resetloadingAllMatching = falseand kill the spinner mid-operation. The guard blocks the watcher until the bus handler'sfinallyblock clears it.3.
loadEnvelopes(queryOverride)fallback. Accepts an optional parameter. The bus handler passes it; normal watcher calls pass nothing and fall back tothis.searchQuery.Changes
SearchMessages.vueselectAllMatching()emitssearch-changed+select-all-matching(query)closeSearchModal()emitssearch-changedfor normal searchfilterData.mentionsreturns''instead offalseMailboxThread.vueonSelectAllMatching(query)forwards the query via the shared busMailbox.vueonBusSelectAllMatching(query)— full mass selection lifecycleloadEnvelopes(queryOverride)— optional query bypass for the bus handlerselectAllLabel— context-aware label:loadingAllMatchingloadingEnvelopesselectModehasFilter!endReachedhasFilter— computed, regex/^match:allof\s*$/selectAllHint— adapts when filter is activesearchQuerywatcher — guarded with_busHandlerActive; resetsendReachedselectMode, disabled duringloadingAllMatchingEnvelopeList.vue_localToggleInProgressguard onselectionandsortedEnvelopswatchers$nextTickto survive Vue's watcher flushEnvelope.vueevent.shiftKeyguard inonSelectMultipleHow to test
Files
src/components/SearchMessages.vuesrc/components/MailboxThread.vuesrc/components/Mailbox.vuesrc/components/EnvelopeList.vuesrc/components/Envelope.vueKnown limitation
Loading iterates over all pages sequentially. A backend bulk endpoint accepting a filter query would remove this bottleneck.