aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorDanilo M. <danix@danix.xyz>2026-08-10 20:43:05 +0200
committerDanilo M. <danix@danix.xyz>2026-08-10 20:43:05 +0200
commited0e085377440a68cef63ade6dc2afd322c22df9 (patch)
treea8b4e5631a6eb5ff56341253f3775b45e3a4c026
parent39a055fef99a3ce6877829753f384843b6a19177 (diff)
downloadqtmaildir-ed0e085377440a68cef63ade6dc2afd322c22df9.tar.gz
qtmaildir-ed0e085377440a68cef63ade6dc2afd322c22df9.zip
feat(view): follow a background sync without a keystroke
The thread list now updates itself when a sync finishes, whether it is empty or populated. New threads appear where the sort puts them, threads that stopped matching leave, and threads whose state changed repaint. Refreshing used to mean re-running the query, which cleared the model, the selection, the message pane and the undo stack, so 0.8.0 declined to do it on a cron timer and asked the user to press Enter instead. The result was a list that quietly disagreed with the database: mail indexed by cron never appeared, and an Unread view read to the end sat empty in front of it. ThreadListModel::reconcile() diffs a result against the current rows by thread id instead, so a surviving thread keeps its row, its persistent index and its loaded replies. Order comes from the result and is never imposed here, which is what makes the sort dropdown authoritative. The undo constraint this was sized around did not exist: no undo entry was ever keyed on a row. ThreadTagCommand stores thread ids and MessageTagCommand stores message ids, and applyTagChange() looks its target up by id, so an entry already survived its rows leaving the view. A thread read out of the current view now leaves the list, which is correct and would otherwise strand the reader, so MessageView grows a notice saying the open thread no longer matches, with a button that re-queries it. Recovery lists the whole conversation, expands it, and restores the message that was on screen rather than reopening at the first one. Ten defects were found building this, nine of them by hand testing: - SyncMonitor::start() polls synchronously, so an idle lock file emits stateChanged(Idle) from inside buildUi() and the first handler to touch a widget segfaults before the window exists. - QTreeView sets a current index when it takes focus with none set, and current drives loading, so new mail opened itself and was marked read without the user having looked at it. Selection is now required. - The notice outlived what it described, both when the pane was blanked and when another message replaced it. - Retiring the "Background sync completed" message left the bar claiming a sync was still running: silent means saying nothing new, not leaving a stale claim on screen. - A thread root sets both the thread id and the message id, so treating the message id as the message-row case discarded it for the commonest way to open a thread. - A freshly queried root does not know its own first message until the tree loads, so recovery selected nothing and left the pane blank. - A user query mid-recovery had its result hijacked by the pending selection. - MessageView emitted the recovery signal with its own members, so a direct connection handed MainWindow references that runCurrentQuery() then cleared by blanking the pane. The ids went empty mid-slot and no recovery ever ran. Every test passed against this, because reaching a slot through invokeMethod copies its arguments. A Qt signal argument is a reference until something copies it. Emitting a member to a slot that can re-enter the emitter is a use-after-write, and it presents as a wrong value rather than as a crash. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
-rw-r--r--CHANGELOG.md19
-rw-r--r--docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md383
-rw-r--r--src/mainwindow.cpp293
-rw-r--r--src/mainwindow.h105
-rw-r--r--src/messageview.cpp56
-rw-r--r--src/messageview.h39
-rw-r--r--src/threadlistmodel.cpp104
-rw-r--r--src/threadlistmodel.h14
-rw-r--r--tests/test_mainwindow.cpp1059
-rw-r--r--tests/test_threadlistmodel.cpp248
10 files changed, 2299 insertions, 21 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md
index a8f0a55..cb213d1 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -11,6 +11,25 @@ point at which they are stable.
## [Unreleased]
+### Changed
+
+- The thread list now follows a background sync on its own. New mail appears
+ where the sort puts it, threads that stopped matching leave, and threads whose
+ state changed repaint, with no keystroke. Previously the status bar asked you
+ to press Enter, because refreshing meant re-running the query, which cleared
+ the list, the selection and the message pane; the list is now reconciled
+ instead, so an expanded thread stays expanded, the selection stays put and the
+ message being read stays on screen. The "Background sync completed" message is
+ gone: a refresh that changes nothing should be invisible.
+
+### Added
+
+- A notice above the message when the thread being read no longer matches the
+ current query, with a button that brings it back. Reading a thread to the end
+ of an Unread view now removes it from the list as it should, and this is the
+ way back to it: the whole thread is listed, and the message that was on screen
+ is re-selected, so returning to reply four of eight lands on reply four.
+
## [0.13.0] - 2026-08-10
The thread list stops being a table. Each thread is a card of three lines,
diff --git a/docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md b/docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md
index 50d6c42..dccea2d 100644
--- a/docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md
+++ b/docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md
@@ -91,11 +91,11 @@ taking that too literally.
| 32 | Esc does not blank the right pane | workflow | XS | **done** |
| 33 | Status bar messages never expire | feedback | S | **done** |
| 34 | No overview of the Maildir itself | information | M | **done** |
-| 35 | No refresh of the thread list after a sync | workflow | M | open |
+| 35 | No refresh of the thread list after a sync | workflow | M | **done** 2026-08-10; the list now follows a sync on its own |
| 36 | `test_mainwindow` cannot reach the worker | testing | S | open, on demand |
| 37 | The worker stalls on a tag edit made during a background sync | correctness | S | **done** |
| 38 | `test_mainwindow` fails when a real sync holds the lock | testing | XS | **done** |
-| 39 | Thread list cannot be sorted by clicking a column header | workflow | S | open |
+| 39 | Thread list cannot be sorted by clicking a column header | workflow | S | **dropped** 2026-08-10; the card list has no column headers to click, and 0.13.0 shipped a sort dropdown instead |
| 40 | No live filter over the current view | workflow | M | open |
| 41 | A message whose HTML body carries a `Content-Id` renders blank | correctness | S | **done** |
| 42 | "Syncing..." says nothing about what is being synced | feedback | S | **done** |
@@ -118,6 +118,8 @@ taking that too literally.
| 59 | Archive and Mark all read shipped with the same icon | presentation | XS | **done** |
| 60 | Next thread dead-ends on the last reply of an expanded thread | defect | XS | **done**; already fixed by 5487d58, see below |
| 61 | `test_mainwindow` fails intermittently, about 1 run in 20 | testing | S | open; predates the card list, reproduced on f72dba9 |
+| 62 | No config option for the date format on a card | presentation | XS | open |
+| 63 | No way to see sent mail, and no filter for it | workflow | S | open |
Sizes are rough: XS under an hour, S a sitting, M a session.
@@ -2175,6 +2177,292 @@ from a current one, so the plumbing for a second concurrent query exists.
- Scroll position likewise.
- A refresh must not re-trigger mark-read for the thread already on screen.
+### Refined by the user, 2026-08-10: new mail never appears at all
+
+The complaint is sharper than "the list is stale". Read every message in an
+Unread view and the list empties. The cron sync then runs, `notmuch new` indexes
+new mail, and **the new messages do not appear**, even though the view is empty,
+nothing is selected, no thread is open and the undo stack has nothing in it. The
+only way to see them is to re-run the query by hand.
+
+**Cause, verified in code, and it is the deliberate choice above meeting its
+worst case.** The app does observe the cron sync:
+`MainWindow::onExternalSyncStateChanged()` (`src/mainwindow.cpp:2140`) is driven
+by `SyncMonitor`'s lock polling and reaches `State::Idle` when the run ends. At
+`src/mainwindow.cpp:2180-2192` it deliberately shows "Background sync completed.
+Press Enter in the query bar to refresh." and calls nothing. The comment there
+gives the reason: `runCurrentQuery()` clears the undo stack, the selection and
+the message pane, which is hostile to fire six times an hour under a reader.
+
+**Every one of those costs is zero in the case the user hit.** There is nothing
+to clear: no undo entries, no selection, no open thread, and the scroll position
+of an empty list is meaningless. So the guard is protecting state that does not
+exist, and the result is an empty Unread view sitting in front of unread mail.
+
+**This makes the item shippable in two stages, and the first is XS.** Refresh on
+external sync completion when the refresh is provably free: the model is empty
+**or** the undo stack is empty and nothing is selected. Fall back to the current
+status-bar message otherwise. That fixes the reported case immediately without
+needing the reconciling refresh, which stays as the M-sized second stage for the
+case where the user does have state worth keeping.
+
+**Constraint on the cheap stage.** "Nothing selected" must be read from the
+selection model, not from `currentRowChanged` state, per `CLAUDE.md`. And the
+condition has to be re-checked at the moment the sync ends rather than when it
+started, since the user may have selected a row during the run.
+
+### Outcome (done, 2026-08-10)
+
+**35a was superseded before it shipped, by the user's own answer to it.** Asked
+whether new mail would accumulate into a POPULATED view, the answer was no, and
+the requirement was restated in full: "changes should be applied automatically,
+without the user needing to refresh the view, whether it's empty or populated",
+new mail appearing at the top as it is fetched, and the message being read not
+disappearing. So the conditional refresh was removed rather than kept beside the
+reconciling one, and `externalRefreshIsFree()` no longer exists. The "press
+Enter in the query bar to refresh" message is gone with it: a refresh that
+changes nothing is invisible, and one that brings mail announces itself by the
+mail appearing.
+
+**The undo constraint this item was sized around turned out not to exist.** The
+text above calls keeping the undo stack "the hard part" and the reason this is
+M-sized. It is not hard, because no undo entry was ever keyed on a row:
+`ThreadTagCommand` stores THREAD ids and `MessageTagCommand` stores MESSAGE ids
+(`src/mainwindow.h`), both re-sending through `sendThreadTagChange()` /
+`sendMessageTagChange()`, and `ThreadListModel::applyTagChange()` looks its
+target up by id and does nothing when the row is absent. An entry therefore
+already survives its rows leaving the view. `runCurrentQuery()` clears the stack
+because a query the USER typed means "show me something else", not because the
+entries would corrupt anything.
+
+**What was built.**
+
+- `ThreadListModel::reconcile()` diffs a result against the current rows by
+ thread id: arrivals inserted, departures removed, survivors keeping their row,
+ their persistent index and their loaded replies. Order comes from the result,
+ never from a rule of the model's own, so the sort the user selected is
+ respected: newest-first puts new mail at the top, oldest-first at the bottom.
+- `MainWindow::refreshCurrentQuery()` re-runs `m_lastQuery` under its own
+ generation, accumulates every batch, and reconciles ONCE at the end. Batch by
+ batch would be wrong in a way that looks right: reconcile decides removals
+ from what the result lacks, so the first batch would delete every row after it
+ and the next would put some back.
+- `onExternalSyncStateChanged()` calls it unconditionally on `Idle`.
+- A stale-thread notice in `MessageView`, shaped like the remote-content bar as
+ the user asked, with recovery that runs `thread:<id>`, expands it and
+ re-selects the message that was on screen.
+
+**Four defects found by the tests, three of them real.**
+
+1. **The first test crashed the constructor.** `SyncMonitor::start()` polls
+ SYNCHRONOUSLY (`src/syncmonitor.cpp:52`), so on an idle lock file it emits
+ `stateChanged(Idle)` from inside `buildUi()`, while the view, the model and
+ the worker are all still null. The old code survived only because reporting
+ to the status bar touches nothing built later; the first handler to
+ dereference a widget segfaults before the window exists.
+2. **A downward-move branch that could never run.** `reconcile()` walks the
+ result front to back, so rows ahead of the target are already final and a
+ misplaced survivor is always pulled FORWARD. The branch was written with the
+ usual `beginMoveRows` +1 adjustment and two mutation tests passed against it
+ being wrong, which is the signal that a probe is not measuring what it
+ claims. Deleted, with `Q_ASSERT(row > target)` recording the invariant.
+3. **A user query hijacked by a pending recovery.** Recovery spans two queued
+ round-trips, so a query typed in the middle of one found its target in the
+ new result and moved the selection there. `runCurrentQuery()` now abandons a
+ pending recovery, and `recoverStaleThread()` sets its target after calling
+ it.
+4. **The stale notice never fired for the reader deepest in a thread.**
+ Selecting a message row CLEARS `m_currentThreadId` and sets
+ `m_currentMessageId` instead, so a notice keyed on the thread alone was
+ silent for exactly the case the user described, reply four of eight. The
+ window remembers the message's thread separately;
+ `ThreadListModel::threadIdForMessage()` cannot help, since it searches the
+ rows and by then the thread has left them.
+
+**A fifth defect, found by the user in hand testing rather than by any test.**
+The notice outlived the message it describes: running a new query blanked the
+pane and left the bar above it, still naming the previous thread, with a button
+offering to recover a thread the user had deliberately navigated away from. The
+bar belongs to the rendered message exactly as the remote-content bar does, and
+`MessageView::clear()` already hides that one for this precise reason; the new
+bar simply was not added beside it. Fixed there, which covers all six paths that
+blank the pane at once, rather than at the query path where it was noticed.
+
+Worth recording because the tests could not have caught it as written: every
+one of them asserted that the notice APPEARS, and none that it goes away. A
+feature's off-switch needs its own test, and "it shows up when it should" passes
+identically whether or not it ever stops showing up.
+
+**A sixth defect, also found by the user in hand testing, and the same mistake
+in a different place.** The status bar sat on "Background sync running..." with
+no sync running. That string is written straight to the label when the lock
+appears, and the "Background sync completed" message on the way out was the only
+thing that ever replaced it; removing that message to make the refresh silent
+left the claim standing indefinitely.
+
+The rule this establishes is worth more than the fix: **silent means saying
+nothing NEW, not leaving a stale claim on screen.** The Idle branch now retires
+its own running message and nothing else, tracked by a flag rather than by
+matching the text, so it cannot overwrite a selection count or a tag result the
+user is reading. Both directions are pinned by mutation: never retiring
+reproduces the reported bug, and always writing the default stamps over the
+selection message.
+
+Note the shape shared with the fifth defect above. Both are a piece of UI state
+that outlived the thing it described, and in both cases the tests asserted only
+that the state APPEARS. An "it goes away" test is a separate test.
+
+**A seventh and eighth defect, one report, and the worse of the two mutates
+mail.** The user came back to the window from another desktop and found the new
+message the refresh had brought in already OPEN in the pane, with the stale
+notice above it still naming the four-message thread they had been reading.
+
+- **Nothing in `MainWindow` selected it.** `QTreeView` gives itself a current
+ index when it takes FOCUS with none set, and current is what drives loading.
+ Probed rather than assumed, because the obvious hypothesis is wrong: inserting
+ rows into an empty view does NOT set current, focusing the view does, which is
+ exactly why the report came with "as I go back to the window from another
+ desktop" attached. Before item 35b this was unreachable, since a populated
+ list always had a current row; a refresh dropping mail into a view the user
+ read empty created the state. The consequence is not cosmetic: opening a
+ message arms the mark-read timer, so a cron sync plus a window switch marked
+ mail read that nobody looked at. `onThreadSelected()` now requires the row to
+ be SELECTED, which every real route (click, arrow key, `selectRowAt`) does and
+ Qt's housekeeping does not.
+- **The notice was correct when raised and became a lie underneath.** It named
+ the thread that was rendered; the auto-open then replaced the pane without
+ touching the bar. `MessageView::clear()` retires it, but selecting a row
+ RE-RENDERS rather than blanking, so that path never ran. Retired in
+ `onThreadSelected()` as well.
+
+The first of these was reported as one bug and is two, and only the second was
+visible on screen. Worth remembering that "the wrong thing is displayed" and
+"the wrong thing happened to the mail" can arrive in the same sentence.
+
+**A ninth defect: recovery brought the thread back collapsed and blank.** The
+user reported it as minor and livable, and it was three faults stacked, each of
+which alone would have produced roughly the symptom they saw.
+
+- **The notice threw away a message id it had.** A thread ROOT sets BOTH
+ `m_currentThreadId` and `m_currentMessageId`, because the root card is the
+ thread's first message and the pane renders exactly that message. The notice
+ read the message id only when the thread id was empty, treating it as the
+ message-row case, so opening a thread the ordinary way lost it and recovery
+ had nothing to reopen.
+- **Recovery never expanded.** It selected the row and returned, so the
+ conversation the user asked to get back to was not on screen. It expands
+ first now, in every branch, which is also what asks the worker for the
+ replies.
+- **A freshly queried root does not know its own first message either.**
+ `MessageIdRole` on a thread row returns `first.messageId`, which is empty
+ until the tree loads, so the root check could not match on the pass that
+ matters and the code fell through to `rowCount(thread) == 0` and returned,
+ selecting nothing. Recovery now selects the thread PROVISIONALLY on that
+ pass, without clearing the target, and refines to the exact reply when the
+ replies arrive.
+
+**One change here is not demonstrated and is recorded as such.** Recovery also
+moved from `setCurrentIndex()` to `selectRowAt()`, on the reasoning that
+`onThreadSelected()` ignores an unselected current index since the auto-open
+fix. A mutation reverting it passes the whole suite: under
+`ExtendedSelection`, `setCurrentIndex()` selects as a side effect, so the two
+are indistinguishable here. It is kept as the honest expression of the intent,
+not as a fix, and nothing should be claimed for it.
+
+**A tenth defect, and the only one in this item that six rounds of reasoning
+failed to find: a dangling reference across a signal.** Recovery brought the
+thread back collapsed with a blank pane. The user reported it three times, each
+time after a fix that was aimed at the wrong thing.
+
+`MessageView` emitted `staleThreadRecoveryRequested(m_staleThreadId,
+m_staleMessageId)`, passing its own members. The connection is direct, so
+`MainWindow::recoverStaleThread()` received REFERENCES to those members. It then
+called `runCurrentQuery()`, which blanks the pane, which calls `setStaleThread()`
+and assigns to exactly those members. From that line onward the slot's own
+parameters read as empty, so `m_recoverThreadId = threadId` stored an empty
+string and `applyPendingRecovery()` returned at its first line, forever. The
+thread was re-queried and expanded correctly, which is why the symptom looked
+like a layout or expansion problem rather than a lifetime one.
+
+**Every existing recovery test passed against it, and could not have failed.**
+They all reach the slot through `QMetaObject::invokeMethod`, which COPIES its
+arguments; the reference never dangles under a test. The defect needed the real
+button and the real signal, which is what the new test uses.
+
+**Six wrong mechanisms were proposed and rejected before the log named this
+one**, each one plausible and each one disproved by a probe rather than by
+argument: `QTreeView::expanded` not re-firing, expansion collapsing when
+children arrive, the multi-row guard blanking the pane, `selectRowAt` not
+clearing the previous selection, a stale `m_refreshGeneration` swallowing the
+result, and `onQueryFinished` not running at all. The thing that ended it was
+instrumenting the running application and reading `RECOVER target set to ` with
+nothing after the `to`, which no amount of reading the code had produced.
+
+**The rule worth keeping: a Qt signal argument is a reference until something
+copies it.** Emitting a member across a direct connection to a slot that can
+reach back and modify that member is a use-after-write, and it presents as the
+value being "wrong" rather than as a crash. Copy at the emit site when the slot
+can plausibly re-enter the emitter.
+
+**A trap the recovery had to handle.** `setThreadMessages()` drops the depth-0
+message because the root card IS the thread's first message, so a reader
+recovering from message one must land on the ROOT row. Looking for it among the
+children finds nothing and leaves the selection nowhere.
+
+**Verification.** 105 tests in `test_mainwindow`, 56 in `test_threadlistmodel`,
+17/17 binaries. Every reconcile test was mutation-checked: an unconditional
+refresh, a skipped move, corrupted index bookkeeping (which trips
+`QAbstractItemModelTester` fatally), a per-batch reconcile, a notice that never
+fires and one that always fires are each caught by a named test. The
+abandoned-recovery test initially passed for the wrong reason, because its
+recovery never reached its target, and was rewritten until it failed against the
+missing guard.
+
+**One measurement worth carrying to item 61.** During this work
+`test_mainwindow` failed three runs in a row on the pair recorded there
+(`anActionOnAMessageRowTagsThatMessageNotTheThread`,
+`aSuccessfulCronSyncDrainsTheEditedAccounts`), then passed twelve consecutive
+runs unchanged. The recorded rate is about one in twenty; a cluster of three
+consecutive failures does not fit an independent one-in-twenty event and
+suggests the trigger is a machine state that persists across runs rather than a
+per-run race.
+
+### Outcome (35a, superseded by the above)
+
+`MainWindow::externalRefreshIsFree()` gates the `State::Idle` branch of
+`onExternalSyncStateChanged()`: it refreshes when the undo stack is empty, the
+selection model reports no selection, and the model holds no rows, and prints
+the existing "press Enter" message otherwise. The M-sized reconciling refresh
+(35b) is untouched and still open.
+
+**The test crashed the constructor, and the crash was real.**
+`SyncMonitor::start()` polls SYNCHRONOUSLY (`src/syncmonitor.cpp:52`), so on a
+machine whose lock file is idle it emits `stateChanged(Idle)` from inside
+`buildUi()` (`src/mainwindow.cpp:535`), while `m_threadView` and `m_model` are
+still null. The old code survived that only because reporting to the status bar
+touches no widget built later; the first handler to dereference a view segfaults
+before the window exists. `externalRefreshIsFree()` returns false on a null view
+or model, which is also correct on the merits: the startup query has not run at
+that point, so there is nothing to refresh.
+
+**The undo check is not redundant with the row check**, and that is asserted
+rather than argued. Removing it alone leaves
+`aCronSyncDoesNotRefreshOverPendingUndo` failing, because an empty model with
+live undo entries is reachable by tagging the last thread out of the current
+view.
+
+**Both guard tests passed before the fix existed**, since nothing refreshed at
+all, so each was verified by mutation: an unconditional `return true` fails both,
+and dropping the undo check fails the undo one.
+
+**One existing test changed, and the change is a narrowing.**
+`aSkippedLocalSyncStillReportsTheOtherRunFinishing` asserted the words
+"Background" in the status bar, and its fixture is an empty list with nothing
+selected, so it now takes the refresh branch and the words never appear. Its
+actual subject is that a handed-back lock is attributed to the other run rather
+than swallowed, so it asserts the query generation instead. Pinning the wording
+there would fail again the next time this decision is revisited.
+
## 36. `test_mainwindow` cannot reach the worker
**Observed:** twice in one session (0.8.0), a defect could not be given a
@@ -2406,6 +2694,15 @@ line and `lockHeldIn()` correctly finds no lock in it.
## 39. Thread list cannot be sorted by clicking a column header
+**Dropped 2026-08-10 (user).** The item asked for a column header to click and
+there is no longer one to click: the card list (items 20 and 53) collapsed the
+five columns into a single column of cards, and the header is gone with them.
+0.13.0 shipped a sort dropdown in the query row, which is the same capability
+reached a different way, so the complaint behind this item is answered and the
+mechanism it proposed is unbuildable. The analysis below is kept because its
+constraints outlived it: the batched-append problem and the sort-on-timestamp
+rule apply to any future sort, including the dropdown's.
+
**Observed (user, 2026-08-05):** "left pane columns order by clicking on the
column header."
@@ -3751,6 +4048,88 @@ initially mistaken for a regression that change had introduced.
**Size: S**, most of it in reproducing reliably rather than in the fix.
+## 62. No config option for the date format on a card
+
+**Observed (user, from the notes):** "option in config file for date format".
+
+**Cause, verified in code.** `CardLayout::formatDate()`
+(`src/cardlayout.cpp:24-30`) is a single unconditional line:
+`QLocale::system().toString(date, QLocale::ShortFormat)`. It reads no setting,
+takes no format argument, and is the only date formatter on a card
+(`src/carddelegate.cpp:176` is its only caller besides
+`CardLayout`'s own `widestDate`). `Config` parses no date key: the `[general]`
+keys it reads are `notmuch_config`, `startup_query`, `message_zoom`,
+`completion_on_focus`, `toolbar_icon_size`, `sync_on_exit` and
+`mark_read_delay_ms` (`src/config.cpp:78-184`). So there is nothing to
+configure, not a setting that is being ignored.
+
+The current behaviour is a deliberate choice rather than an oversight, and the
+comment at `src/cardlayout.cpp:26-28` says why: the system short format is what
+every other application on the desktop shows, and a mail client that disagrees
+looks wrong. This item is about giving the user an override, not about
+replacing that default.
+
+**Approach.** A `[general] date_format` key, empty by default meaning "the
+system short format", otherwise a `QDateTime::toString()` pattern.
+`CardLayout::formatDate()` takes the format as a parameter rather than reading
+`Config` itself, keeping the struct free of dependencies the way it already is,
+and `CardDelegate` passes it down.
+
+**Constraints.**
+
+- **`CardLayout::widestDate()` reserves the date column's width and must agree
+ with whatever the format produces**, or a long custom pattern is elided or
+ overlaps. It currently computes the widest string the short format can return;
+ with a custom pattern it has to measure that pattern instead.
+- An unparseable or absurd pattern must not blank the date. `toString()` with a
+ pattern containing no field returns the pattern itself verbatim, so validate
+ in `Config` and fall back to the system format with a problem reported, the
+ way `message_zoom` and `toolbar_icon_size` already do.
+- Document all three states in the README's `[general]` block: absent, empty,
+ and a pattern.
+
+**Size: XS.** One key, one parameter, one width calculation.
+
+## 63. No way to see sent mail, and no filter for it
+
+**Observed (user, from the notes):** "Sent mail filter".
+
+**Cause, verified in code, and it is not one missing feature but two.**
+
+- **Nothing in the codebase knows what "sent" means.** `Account` carries
+ `name`, `address`, `maildir`, `drafts`, `label`, `channel` and `color`
+ (`src/config.cpp:254-270`); there is no `sent` field, and no query anywhere
+ composes one. `Account::scopedQuery()` (`src/config.cpp:42-45`) scopes by
+ `path:"<maildir>/**"`, which covers a sent folder only in the sense that it
+ covers everything in the account.
+- **The saved-query mechanism could express it today and nothing ships one.**
+ `[queries]` is read wholesale from `childKeys()` (`src/config.cpp:291-297`),
+ so a user can already write `Sent = tag:sent` by hand. The README's example
+ block (`README.md:178-181`) offers Inbox, Unread and Important and no Sent, so
+ nothing points the user at it.
+
+Which of the two this item is depends on a decision the notes do not make:
+whether "sent" is a **notmuch tag** the user's own filters apply, or a
+**maildir path** per account. If it is a tag, this is a documentation and
+defaults change and it is XS. If it is a path, `Account` needs a `sent` key
+beside `drafts`, and the query has to be composed per account, which is where
+the S comes from.
+
+**Approach, pending that decision.** Ask the user first which their setup
+already produces. Their mail is filtered outside qtmaildir (`assets/mailsync.sh`
+is `mbsync` plus `notmuch new`), so the answer is a property of their existing
+filters, not something to design here.
+
+**Constraints.**
+
+- **Do not invent a tag qtmaildir applies itself.** v1 is read-and-organize;
+ nothing here sends mail, so nothing here can know a message was sent except by
+ where it landed or what tagged it.
+- If it becomes a per-account key, it composes with `scopedQuery()` and must not
+ bypass it, or a Sent view in one account shows another account's mail.
+
+**Size: S**, and XS if the answer is "it is a tag".
+
## Deferred, unsized, or split out
Items noted while triaging but not part of the original list. Same numbering
diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp
index b7efec5..bc5fa92 100644
--- a/src/mainwindow.cpp
+++ b/src/mainwindow.cpp
@@ -626,6 +626,8 @@ void MainWindow::buildUi()
this, [this](const QString &text) { m_statusLabel->setText(text); });
connect(m_messageView, &MessageView::queryRequested,
this, &MainWindow::onPlaceholderQueryRequested);
+ connect(m_messageView, &MessageView::staleThreadRecoveryRequested,
+ this, &MainWindow::recoverStaleThread);
m_splitter = new QSplitter(Qt::Horizontal, central);
m_splitter->addWidget(m_threadView);
@@ -867,6 +869,7 @@ void MainWindow::registerActions()
// CLAUDE.md.
m_currentThreadId.clear();
m_currentMessageId.clear();
+ m_currentMessageThreadId.clear();
m_messageView->clear();
showPlaceholderPane();
m_markReadTimer->stop();
@@ -901,6 +904,7 @@ void MainWindow::registerActions()
m_currentThreadId.clear();
m_currentMessageId.clear();
+ m_currentMessageThreadId.clear();
m_messageView->clear();
showPlaceholderPane();
m_markReadTimer->stop();
@@ -1480,6 +1484,15 @@ void MainWindow::runCurrentQuery()
// Kept so loadThread() can work out which messages of a thread matched.
m_lastQuery = query;
+ // A query the user ran abandons any recovery still in flight. Recovery
+ // spans two round-trips, so a query typed in the middle of one would
+ // otherwise have its result hijacked: the pending selection finds its
+ // thread in a result the user asked for something else from, and the view
+ // jumps. recoverStaleThread() sets the target AFTER calling this, so its
+ // own query does not clear it.
+ m_recoverThreadId.clear();
+ m_recoverMessageId.clear();
+
++m_generation;
m_model->clear();
m_messageView->clear();
@@ -1513,6 +1526,17 @@ void MainWindow::onThreadsReady(const QVector<ThreadSummary> &threads,
{
if (generation != m_generation)
return; // Superseded by a newer query.
+
+ // A refresh accumulates instead of appending. Its batches must not reach
+ // the model one at a time: reconcile() decides what to REMOVE from what the
+ // result does not contain, so applying the first batch alone would delete
+ // every row after it, then the next batch would put some back. The list
+ // would churn and every expanded thread would collapse.
+ if (generation == m_refreshGeneration) {
+ m_refreshThreads.append(threads);
+ return;
+ }
+
m_model->appendBatch(threads);
}
@@ -1520,6 +1544,30 @@ void MainWindow::onQueryFinished(int total, quint64 generation)
{
if (generation != m_generation)
return;
+
+ // The refresh's result is complete only now, so this is where it lands.
+ // One reconcile for the whole set, not one per batch.
+ if (generation == m_refreshGeneration) {
+ m_refreshGeneration = 0;
+ m_model->reconcile(m_refreshThreads);
+ m_refreshThreads.clear();
+
+ // The count in the status bar describes the current view and has just
+ // changed, but a refresh is meant to be silent, so it updates the
+ // FALLBACK text without stamping over whatever the bar is showing.
+ m_defaultStatus = tr("%n thread(s)", "", total);
+
+ // A refresh leaves the view complete exactly as a query does: every
+ // matching row is present, so view-wide actions stay honest.
+ m_queryComplete = true;
+ updateViewWideActions();
+
+ // The open thread may have stopped matching, which the user has to be
+ // told about: the pane keeps rendering it while the list no longer
+ // offers it anywhere.
+ updateStaleThreadNotice();
+ return;
+ }
// The query's own result is what the bar says when nothing more pressing
// is happening, so a transient message falls back to it rather than to
// nothing.
@@ -1530,6 +1578,11 @@ void MainWindow::onQueryFinished(int total, quint64 generation)
// a thing that can honestly be acted on.
m_queryComplete = true;
updateViewWideActions();
+
+ // A recovery's own thread:<id> query landing. The rows exist now, so the
+ // thread can be expanded; the message inside it is selected once its
+ // replies arrive.
+ applyPendingRecovery();
}
void MainWindow::updateViewWideActions()
@@ -1692,6 +1745,7 @@ void MainWindow::onSelectionChanged()
m_markReadThreadId.clear();
m_currentThreadId.clear();
m_currentMessageId.clear();
+ m_currentMessageThreadId.clear();
m_messageView->clear();
showPlaceholderPane();
}
@@ -1702,6 +1756,29 @@ void MainWindow::onThreadSelected(const QModelIndex &current,
if (!current.isValid())
return;
+ // A current index the user did not put there. QTreeView gives itself one
+ // when it takes FOCUS with none set (verified against Qt 6.11: inserting
+ // rows does not do it, focusing the view does), and it sets current WITHOUT
+ // selecting. Before item 35b nothing could reach that state, because a
+ // populated list always had a current row; now a refresh can drop mail into
+ // a view the user read empty, and coming back to the window from another
+ // desktop would open the new message and mark it read two seconds later
+ // without them ever having looked at it.
+ //
+ // Every real route here (a click, an arrow key, selectRowAt) selects the
+ // row as well, so requiring a selection separates the user's intent from
+ // Qt's housekeeping without weakening any of them.
+ if (!m_threadView->selectionModel()->isSelected(current))
+ return;
+
+ // The notice belongs to whatever the pane is showing, and it is about to
+ // show something else. Retired here rather than only in MessageView::clear()
+ // because selecting a row RE-RENDERS the pane instead of blanking it, so
+ // the bar would otherwise sit over a message it does not describe. That is
+ // the second half of the reported defect: the pane had moved on and the
+ // notice had not.
+ m_messageView->setStaleThread(QString(), QString());
+
// A selection spanning more than one row is aimed at a bulk action, not at
// reading. current follows the keyboard cursor as the selection extends, so
// without this every row swept through would be rendered and, worse,
@@ -1724,6 +1801,7 @@ void MainWindow::onThreadSelected(const QModelIndex &current,
m_markReadThreadId.clear();
m_currentThreadId.clear();
m_currentMessageId.clear();
+ m_currentMessageThreadId.clear();
m_messageView->clear();
showPlaceholderPane();
return;
@@ -1747,6 +1825,9 @@ void MainWindow::onThreadSelected(const QModelIndex &current,
m_currentThreadId.clear();
m_currentMessageId = node.messageId;
+ // Remembered for the stale notice: the pane shows one message, but the
+ // thread it came from is what the refreshed list is checked against.
+ m_currentMessageThreadId = node.threadId;
m_messageView->setTags(node.tags);
QMetaObject::invokeMethod(m_worker, "loadMessage", Qt::QueuedConnection,
Q_ARG(QString, node.messageId),
@@ -1780,6 +1861,7 @@ void MainWindow::onThreadSelected(const QModelIndex &current,
}
m_currentMessageId.clear();
+ m_currentMessageThreadId.clear();
QMetaObject::invokeMethod(m_worker, "loadThread", Qt::QueuedConnection,
Q_ARG(QString, m_currentThreadId),
Q_ARG(QString, m_lastQuery),
@@ -1836,6 +1918,10 @@ void MainWindow::onThreadTreeLoaded(const QVector<MessageNode> &nodes,
// expansions can be in flight at once, and pairing them by order would
// attach one thread's replies to the other.
m_model->setThreadMessages(nodes.first().threadId, nodes);
+
+ // A stale-thread recovery waits for exactly this: the message it wants to
+ // select does not exist as a row until the replies land.
+ applyPendingRecovery();
}
void MainWindow::onThreadLoaded(const QVector<MessageRef> &messages,
@@ -2137,6 +2223,169 @@ void MainWindow::onTagsApplied(const TagChange &change)
}
}
+void MainWindow::refreshCurrentQuery()
+{
+ // The null guard is not defensive padding, it is a reachable path found by
+ // this item's own test crashing the constructor. SyncMonitor::start() polls
+ // SYNCHRONOUSLY (src/syncmonitor.cpp:52), so a machine whose lock file is
+ // idle at that moment emits stateChanged(Idle) from inside buildUi(), while
+ // m_model and the worker are still null. Nothing to refresh at that point
+ // anyway: the startup query has not run.
+ if (!m_model || !m_worker)
+ return;
+
+ // m_lastQuery, not the text in the query bar: the bar holds whatever the
+ // user has typed since, which may be a query they never ran. Refreshing to
+ // that would execute a search they did not ask for.
+ if (m_lastQuery.isEmpty())
+ return;
+
+ // Nothing is cleared. No m_model->clear(), no m_undoStack.clear(), no
+ // m_messageView->clear(): that list is exactly what runCurrentQuery()
+ // destroys and what makes it unusable on a cron timer.
+ m_refreshGeneration = ++m_generation;
+ m_refreshThreads.clear();
+
+ const auto sort = m_sortOrder->currentIndex() == 1
+ ? NotmuchWorker::OldestFirst
+ : NotmuchWorker::NewestFirst;
+ QMetaObject::invokeMethod(m_worker, "runQuery", Qt::QueuedConnection,
+ Q_ARG(QString, m_lastQuery),
+ Q_ARG(quint64, m_refreshGeneration),
+ Q_ARG(NotmuchWorker::SortOrder, sort));
+}
+
+void MainWindow::updateStaleThreadNotice()
+{
+ // Which thread the pane is showing depends on what was selected: a thread
+ // row sets m_currentThreadId, a message row clears it and sets
+ // m_currentMessageId instead, so the message case has to be resolved back
+ // to its thread. Reading only m_currentThreadId would leave a reader who is
+ // three replies deep with no notice at all, which is the commonest way to
+ // be deep in a thread in the first place.
+ // The message id is carried whenever there IS one, whichever row kind put
+ // it there. A thread ROOT sets both: the root card is the thread's first
+ // message and the pane renders that message alone, so treating the message
+ // id as the message-row case only threw it away for the commonest way to
+ // open a thread, and recovery then had nothing to reopen.
+ QString threadId = m_currentThreadId;
+ const QString messageId = m_currentMessageId;
+ if (threadId.isEmpty())
+ threadId = m_currentMessageThreadId;
+
+ if (threadId.isEmpty()) {
+ m_messageView->setStaleThread(QString(), QString());
+ return;
+ }
+
+ // Present means matching: the model holds exactly the query's result after
+ // a reconcile.
+ for (int row = 0; row < m_model->rowCount(QModelIndex()); ++row) {
+ if (m_model->threadAt(row).threadId == threadId) {
+ m_messageView->setStaleThread(QString(), QString());
+ return;
+ }
+ }
+
+ m_messageView->setStaleThread(threadId, messageId);
+}
+
+void MainWindow::recoverStaleThread(const QString &threadId,
+ const QString &messageId)
+{
+ if (threadId.isEmpty())
+ return;
+
+ // thread:<id> lists the WHOLE conversation rather than the single message,
+ // which is what the user asked for: eight messages, with the fourth
+ // selected, matching what the pane already shows.
+ m_queryEdit->setText(QStringLiteral("thread:%1").arg(threadId));
+ runCurrentQuery();
+
+ // Set AFTER the query, which clears any pending recovery: this one is the
+ // query's own reason for running and must survive it.
+ //
+ // Remembered across the two queued round-trips this takes: the query has to
+ // come back before the thread can be expanded, and the expansion before the
+ // message row exists to select.
+ m_recoverThreadId = threadId;
+ m_recoverMessageId = messageId;
+}
+
+void MainWindow::applyPendingRecovery()
+{
+ if (m_recoverThreadId.isEmpty())
+ return;
+
+ for (int row = 0; row < m_model->rowCount(QModelIndex()); ++row) {
+ const QModelIndex thread = m_model->index(row, 0, QModelIndex());
+ if (m_model->threadAt(row).threadId != m_recoverThreadId)
+ continue;
+
+ // Expanded in every case, and FIRST. The user was reading a
+ // conversation, so bringing it back collapsed hides the thing they
+ // asked to get back to, whether their message was the root or a reply.
+ // Expanding is also what asks the worker for the replies, so it has to
+ // happen before any attempt to find one.
+ m_threadView->expand(thread);
+
+ // The thread's first message IS the root card rather than a child row:
+ // setThreadMessages drops depth 0 because the root stands for it, so
+ // looking for it among the children finds nothing and the selection
+ // would silently land nowhere.
+ //
+ // selectRowAt(), not setCurrentIndex(): a current index without a
+ // selection is what QTreeView sets by itself on focus, and
+ // onThreadSelected() deliberately ignores that, so pointing at the row
+ // renders nothing and leaves the pane blank.
+ if (m_recoverMessageId.isEmpty()
+ || m_model->data(thread, ThreadListModel::MessageIdRole).toString()
+ == m_recoverMessageId) {
+ selectRowAt(thread);
+ m_recoverThreadId.clear();
+ m_recoverMessageId.clear();
+ return;
+ }
+
+ // A reply cannot be selected until the replies exist. The expand above
+ // asked for them, and this runs again when they arrive.
+ //
+ // The thread is selected NOW rather than waiting, because a freshly
+ // queried row does not know its own first message either: the root's
+ // MessageIdRole is empty until the tree loads
+ // (`src/threadlistmodel.cpp`), so the root check above cannot match yet
+ // and returning here would leave the user looking at a collapsed thread
+ // and a blank pane until the replies happen to arrive. Selecting the
+ // thread renders its first message immediately, which is the right
+ // answer outright when that is what they were reading, and is refined
+ // to the correct reply on the next pass when it is not.
+ //
+ // The target is deliberately NOT cleared: this pass is provisional.
+ if (m_model->rowCount(thread) == 0) {
+ selectRowAt(thread);
+ return;
+ }
+
+ for (int child = 0; child < m_model->rowCount(thread); ++child) {
+ const QModelIndex reply = m_model->index(child, 0, thread);
+ if (m_model->messageAt(reply).messageId != m_recoverMessageId)
+ continue;
+ selectRowAt(reply);
+ m_recoverThreadId.clear();
+ m_recoverMessageId.clear();
+ return;
+ }
+
+ // The thread came back without the message: it was deleted, or moved
+ // between accounts. Land on the thread rather than leaving the user
+ // with nothing selected.
+ selectRowAt(thread);
+ m_recoverThreadId.clear();
+ m_recoverMessageId.clear();
+ return;
+ }
+}
+
void MainWindow::onExternalSyncStateChanged(SyncMonitor::State state)
{
if (state == SyncMonitor::State::Running) {
@@ -2154,6 +2403,7 @@ void MainWindow::onExternalSyncStateChanged(SyncMonitor::State state)
m_externalSyncBusy = true;
updateSyncControls();
m_statusLabel->setText(tr("Background sync running..."));
+ m_announcedExternalSync = true;
return;
}
@@ -2177,19 +2427,40 @@ void MainWindow::onExternalSyncStateChanged(SyncMonitor::State state)
m_externalSyncBusy = false;
updateSyncControls();
- // Deliberately reports rather than refreshes. runCurrentQuery() clears the
- // undo stack, the selection and the message pane, which is right for a
- // query the user typed and hostile for one fired by a cron timer: with a
- // sync every ten minutes it would discard undo history and close the thread
- // being read, up to six times an hour, with no action from the user.
+ // Refreshes, unconditionally, and says nothing about it.
+ //
+ // 0.8.0 refused to refresh here because runCurrentQuery() clears the undo
+ // stack, the selection and the message pane, which is right for a query the
+ // user typed and hostile for one fired by a cron timer. The status bar
+ // asked the user to press Enter instead. That made the list quietly stale:
+ // new mail indexed by cron never appeared, and an Unread view read to the
+ // end stayed empty in front of it.
+ //
+ // The answer is not to weigh the cost, it is to remove it.
+ // refreshCurrentQuery() reconciles the result into the model instead of
+ // resetting it, so a surviving thread keeps its row, its expansion and its
+ // selection, and the message being read stays on screen. Nothing has to be
+ // preserved by declining to run.
//
- // Unknown is not worth reporting either. It means the lock table could not
- // be read, so nothing was observed, and "sync finished" would be a claim
- // this cannot support.
+ // No status message: a refresh that changes nothing must be invisible, and
+ // one that adds mail is announced by the mail appearing. Six "sync
+ // completed" messages an hour are noise reporting the expected.
+ //
+ // Unknown is not refreshed. It means the lock table could not be read, so
+ // no sync was observed, and refreshing on it would re-query on every failed
+ // poll rather than after a sync.
+ // Retire our own running message, and only that one. The refresh below says
+ // nothing, which is right for a sync that changed nothing, but "says
+ // nothing" must not mean "leaves 'Background sync running...' on screen
+ // after it stopped". Anything else in the bar belongs to the user (a
+ // selection count, a tag result) and is left alone.
+ if (m_announcedExternalSync) {
+ m_announcedExternalSync = false;
+ m_statusLabel->setText(m_defaultStatus);
+ }
+
if (state == SyncMonitor::State::Idle) {
- showTransientStatus(
- tr("Background sync completed. Press Enter in the query bar to "
- "refresh."));
+ refreshCurrentQuery();
// Item 54. A cron sync carries the edits to the mail store exactly as a
// local one does, so the count it cleared has to be cleared here too.
diff --git a/src/mainwindow.h b/src/mainwindow.h
index ccac5ad..a7ea5c3 100644
--- a/src/mainwindow.h
+++ b/src/mainwindow.h
@@ -155,6 +155,16 @@ public:
/// reopened, so a test standing in for the worker needs the current value.
quint64 statsGenerationForTesting() const { return m_statsGeneration; }
+ /// Whether a stale-thread recovery is still waiting for its result.
+ ///
+ /// A test seam. The recovery target is cleared as a matter of course by any
+ /// query the user runs, so "is it still set immediately after the button"
+ /// is the only way to see that it survived the slot that set it.
+ bool hasPendingRecoveryForTesting() const
+ {
+ return !m_recoverThreadId.isEmpty();
+ }
+
protected:
void closeEvent(QCloseEvent *event) override;
@@ -165,6 +175,23 @@ protected:
private slots:
void runCurrentQuery();
+
+ /// Brings back a thread that stopped matching, and restores the reader's
+ /// place inside it.
+ ///
+ /// Runs `thread:<id>` so the whole conversation is listed rather than the
+ /// single message, then expands it and selects `messageId` once the rows
+ /// exist. Both steps are queued round-trips to the worker, so the ids are
+ /// remembered in m_recoverThreadId / m_recoverMessageId and acted on as the
+ /// replies arrive.
+ ///
+ /// A slot because MessageView's notice connects to it, and because the
+ /// sequencing above is only testable by driving it through the same entry
+ /// point the button uses.
+ void recoverStaleThread(const QString &threadId, const QString &messageId);
+
+ /// Selects the remembered message once its thread's rows have loaded.
+ void applyPendingRecovery();
void onThreadsReady(const QVector<ThreadSummary> &threads, quint64 generation);
void onQueryFinished(int total, quint64 generation);
void onThreadSelected(const QModelIndex &current, const QModelIndex &previous);
@@ -389,6 +416,27 @@ private:
/// Sends every edit held while the lock was busy, oldest first.
void flushHeldEdits();
+ /// Re-runs the current query and reconciles the result into the model.
+ ///
+ /// The non-destructive counterpart to `runCurrentQuery()`, and what a sync
+ /// fires: nothing is cleared, so the selection, the expanded threads, the
+ /// undo stack and the message being read all survive. New threads appear
+ /// where the sort puts them and threads that stopped matching leave.
+ ///
+ /// Does nothing when no query has run yet, since there is nothing to
+ /// re-run.
+ void refreshCurrentQuery();
+
+ /// Shows or hides the message pane's "no longer matches" notice.
+ ///
+ /// Called after a refresh, which is the only thing that can remove a row
+ /// from under a reader. A thread read out of an Unread view is the ordinary
+ /// case: the pane keeps rendering it, correctly, while the list no longer
+ /// offers it anywhere, and without this the message quietly becomes an
+ /// orphan with no route back.
+ void updateStaleThreadNotice();
+
+
/// A tag change not yet sent to the worker, because a sync held the write
/// lock when the user made it.
///
@@ -513,6 +561,63 @@ private:
/// and reopened, so an old answer cannot fill in a newer dialog.
quint64 m_statsGeneration = 0;
+ /// The generation of a REFRESH query, run after a sync to bring the list
+ /// up to date without disturbing it.
+ ///
+ /// Numbered from the same counter as an ordinary query, so a refresh and a
+ /// user query can never share an id, but tracked separately because the two
+ /// consume their results differently: an ordinary query appends into a
+ /// cleared model as batches arrive, while a refresh accumulates every batch
+ /// and reconciles once at the end. Zero when no refresh is in flight.
+ ///
+ /// A user query started while a refresh is running silently supersedes it:
+ /// the refresh's batches are still collected but its result is dropped, for
+ /// the same reason the generation counter exists at all. Reconciling it
+ /// would fight the query the user just typed.
+ quint64 m_refreshGeneration = 0;
+
+ /// Threads collected from a refresh query, complete only once its
+ /// queryFinished arrives.
+ ///
+ /// Held rather than applied per batch because reconcile() needs the WHOLE
+ /// result to tell a thread that stopped matching from one that simply has
+ /// not arrived yet. Reconciling batch by batch would delete every row the
+ /// first batch did not contain, emptying the list and refilling it, which
+ /// is the reset this exists to avoid.
+ QVector<ThreadSummary> m_refreshThreads;
+
+ /// The thread and message a stale-thread recovery is waiting to select.
+ ///
+ /// Recovery spans two queued round-trips (the query, then the reply walk),
+ /// so the target cannot be a local variable. Cleared once the selection
+ /// lands, or by any query the user runs in the meantime: that is them
+ /// choosing to go somewhere else, and restoring a selection into a result
+ /// they did not ask for would yank the view.
+ QString m_recoverThreadId;
+ QString m_recoverMessageId;
+
+ /// The thread the pane's current MESSAGE belongs to.
+ ///
+ /// Selecting a message row clears m_currentThreadId (the pane shows one
+ /// message, not a conversation), so without this a reader three replies
+ /// deep has no thread to check against the refreshed list, and the stale
+ /// notice never appears for them. Not obtainable from the model after the
+ /// fact: ThreadListModel::threadIdForMessage() searches the rows, and by
+ /// the time this is needed the thread has left them.
+ QString m_currentMessageThreadId;
+
+ /// True while the status bar is showing this window's own "Background sync
+ /// running..." message.
+ ///
+ /// A refresh after a cron sync is deliberately silent, so it writes nothing
+ /// to the bar. That left the running message standing after the sync
+ /// finished, because the "completed" message it replaced was the only thing
+ /// that ever cleared it. Silence means saying nothing NEW, not leaving a
+ /// stale claim on screen: this marks the one string the Idle branch is
+ /// entitled to retire, so it cannot overwrite a selection count or anything
+ /// else the user is actually reading.
+ bool m_announcedExternalSync = false;
+
/// Holds the sync log and its close button, so the pane can be dismissed.
QWidget *m_syncLogPane = nullptr;
QPlainTextEdit *m_syncLog = nullptr;
diff --git a/src/messageview.cpp b/src/messageview.cpp
index b804d37..b6c3fa3 100644
--- a/src/messageview.cpp
+++ b/src/messageview.cpp
@@ -188,6 +188,47 @@ MessageView::MessageView(QWidget *parent)
blockedRow->addWidget(m_loadRemoteButton);
blockedRow->addStretch();
+ // The stale-thread notice, deliberately the same shape as the row above:
+ // a sentence and a button, above the message, leaving it readable. The
+ // user asked for this rather than for a dialog, and a dialog would be
+ // wrong anyway, since nothing here needs an answer before the message can
+ // be read.
+ m_staleBar = new QWidget(this);
+ m_staleBar->setObjectName(QStringLiteral("staleThreadBar"));
+ m_staleLabel = new QLabel(
+ tr("This thread no longer matches the current query."), m_staleBar);
+ m_staleButton = new QPushButton(tr("Show it anyway"), m_staleBar);
+ m_staleButton->setObjectName(QStringLiteral("staleThreadButton"));
+ connect(m_staleButton, &QPushButton::clicked, this, [this] {
+ if (m_staleThreadId.isEmpty())
+ return;
+
+ // COPIES, not the members themselves, and this is load-bearing rather
+ // than tidy. A direct connection passes these by reference all the way
+ // into MainWindow::recoverStaleThread(), which calls runCurrentQuery(),
+ // which blanks the pane, which calls setStaleThread() and assigns to
+ // the very members those references name. The ids then read as empty
+ // for the rest of the slot, so the recovery target was stored as an
+ // empty string and nothing was ever recovered: the thread came back
+ // collapsed with a blank pane, which is exactly the reported symptom.
+ //
+ // Invisible to a test that reaches the slot through invokeMethod,
+ // because that copies the arguments; it needs the real signal.
+ const QString threadId = m_staleThreadId;
+ const QString messageId = m_staleMessageId;
+
+ // The message on screen goes with the request. Recovering the thread
+ // alone would reopen it at its first message, and the user was reading
+ // message four of eight.
+ emit staleThreadRecoveryRequested(threadId, messageId);
+ });
+ auto *staleRow = new QHBoxLayout(m_staleBar);
+ staleRow->setContentsMargins(0, 0, 0, 0);
+ staleRow->addWidget(m_staleLabel);
+ staleRow->addWidget(m_staleButton);
+ staleRow->addStretch();
+ m_staleBar->hide();
+
m_attachmentBar = new QWidget(this);
m_attachmentBar->setObjectName(QStringLiteral("attachmentBar"));
new QHBoxLayout(m_attachmentBar);
@@ -200,6 +241,7 @@ MessageView::MessageView(QWidget *parent)
auto *layout = new QVBoxLayout(this);
layout->addLayout(headerRow);
layout->addLayout(blockedRow);
+ layout->addWidget(m_staleBar);
layout->addWidget(m_view, 1);
layout->addWidget(m_attachmentBar);
layout->addWidget(m_tagStrip);
@@ -279,6 +321,12 @@ void MessageView::clear()
m_blockedLabel->hide();
m_loadRemoteButton->hide();
+ // The stale notice describes the message that WAS rendered, so it goes with
+ // it, for the same reason as the blocked-content bar above. Left behind, it
+ // sits over a blank pane naming a thread that is no longer shown, and its
+ // button offers to recover a thread the user has navigated away from.
+ setStaleThread(QString(), QString());
+
// clear() does not go through render(), so the bar has to be emptied
// here or the previous thread's attachments stay offered.
rebuildAttachmentBar();
@@ -654,6 +702,14 @@ void MessageView::saveAttachment(const Attachment &attachment)
emit statusMessage(tr("Saved %1").arg(written));
}
+void MessageView::setStaleThread(const QString &threadId,
+ const QString &messageId)
+{
+ m_staleThreadId = threadId;
+ m_staleMessageId = messageId;
+ m_staleBar->setVisible(!threadId.isEmpty());
+}
+
void MessageView::toggleHtml()
{
const bool anyHtml = std::any_of(
diff --git a/src/messageview.h b/src/messageview.h
index 135c85d..63fd6d8 100644
--- a/src/messageview.h
+++ b/src/messageview.h
@@ -104,6 +104,28 @@ public:
qreal zoomFactor() const;
void setZoomFactor(qreal factor);
+ /// Shows or hides the notice saying the rendered thread no longer matches
+ /// the current query.
+ ///
+ /// Modelled on the remote-content bar rather than on a dialog: the message
+ /// stays readable underneath, and the way back is one click. Passing an
+ /// empty id hides it.
+ ///
+ /// The pane does not decide this for itself. It renders whatever it was
+ /// last given and has no idea what the thread list holds, so the window
+ /// tells it after a refresh.
+ /// `messageId` is the message on screen, empty when a whole thread is
+ /// rendered. It rides along so recovery can restore the reader's place
+ /// rather than reopening the thread at its first message.
+ void setStaleThread(const QString &threadId, const QString &messageId);
+
+ /// The thread the stale notice offers to bring back, empty when hidden.
+ QString staleThreadId() const { return m_staleThreadId; }
+
+ /// The message the stale notice would restore, empty when a whole thread
+ /// is rendered or the notice is hidden.
+ QString staleMessageId() const { return m_staleMessageId; }
+
public slots:
void toggleHtml();
void loadRemoteContent();
@@ -125,6 +147,16 @@ signals:
/// app" is a boundary worth keeping shut rather than arguing about.
void queryRequested(const QString &query);
+ /// The user asked to see a thread that stopped matching the current query.
+ ///
+ /// Carries the thread id and the message that was on screen, because
+ /// recovering the thread alone would land the user on its first message
+ /// rather than the one they were reading. The window runs the query,
+ /// expands the thread and restores the selection; the view knows none of
+ /// that.
+ void staleThreadRecoveryRequested(const QString &threadId,
+ const QString &messageId);
+
protected:
/// Turns Ctrl+wheel over the body into zoom, and Ctrl+middle-click into a
/// reset. Both events are delivered to the web view's internal QQuickWidget
@@ -185,6 +217,13 @@ private:
QLabel *m_headerLabel = nullptr;
QLabel *m_blockedLabel = nullptr;
QPushButton *m_loadRemoteButton = nullptr;
+
+ /// The stale-thread notice and the thread it offers to restore.
+ QWidget *m_staleBar = nullptr;
+ QLabel *m_staleLabel = nullptr;
+ QPushButton *m_staleButton = nullptr;
+ QString m_staleThreadId;
+ QString m_staleMessageId;
QPushButton *m_detailsButton = nullptr;
QWidget *m_attachmentBar = nullptr;
TagStrip *m_tagStrip = nullptr;
diff --git a/src/threadlistmodel.cpp b/src/threadlistmodel.cpp
index bdc7e96..21a6378 100644
--- a/src/threadlistmodel.cpp
+++ b/src/threadlistmodel.cpp
@@ -18,6 +18,8 @@
#include "threadlistmodel.h"
+#include <QSet>
+
#include <QBrush>
#include <QFont>
#include <QGuiApplication>
@@ -551,6 +553,108 @@ void ThreadListModel::appendBatch(const QVector<ThreadSummary> &batch)
endInsertRows();
}
+void ThreadListModel::reconcile(const QVector<ThreadSummary> &threads)
+{
+ // Removals first, walking BACKWARDS. Each beginRemoveRows renumbers
+ // everything after it, so a forward walk would delete by stale indices; a
+ // backward one only ever disturbs rows it has already passed.
+ //
+ // One signal per contiguous run rather than per row: a view rebuilds its
+ // selection and its persistent indexes on every one, and an Unread view
+ // emptied by a sync can drop dozens at once.
+ QSet<QString> wanted;
+ wanted.reserve(threads.size());
+ for (const ThreadSummary &summary : threads)
+ wanted.insert(summary.threadId);
+
+ for (int row = m_threads.size() - 1; row >= 0; --row) {
+ if (wanted.contains(m_threads.at(row).summary.threadId))
+ continue;
+ int first = row;
+ while (first > 0
+ && !wanted.contains(m_threads.at(first - 1).summary.threadId))
+ --first;
+ beginRemoveRows({}, first, row);
+ m_threads.remove(first, row - first + 1);
+ endRemoveRows();
+ row = first;
+ }
+
+ // What survived, by id, so the second pass can tell an arrival from a
+ // thread that merely moved.
+ QHash<QString, int> present;
+ present.reserve(m_threads.size());
+ for (int row = 0; row < m_threads.size(); ++row)
+ present.insert(m_threads.at(row).summary.threadId, row);
+
+ // Insertions, forwards, at the position the RESULT gives them. Walking the
+ // result in order means each new thread is placed against rows already
+ // agreed on, so the model ends in the result's order without this having to
+ // know what that order means.
+ for (int target = 0; target < threads.size(); ++target) {
+ const ThreadSummary &summary = threads.at(target);
+ const auto it = present.constFind(summary.threadId);
+
+ if (it == present.constEnd()) {
+ const int at = qMin(target, m_threads.size());
+ beginInsertRows({}, at, at);
+ m_threads.insert(at, ThreadNode{ summary, {}, {}, false });
+ endInsertRows();
+
+ // Every later row shifted by one, and the map is read again on the
+ // next iteration.
+ for (auto entry = present.begin(); entry != present.end(); ++entry) {
+ if (entry.value() >= at)
+ ++entry.value();
+ }
+ continue;
+ }
+
+ // A survivor that MOVED, which is neither an arrival nor a departure
+ // and is the commonest reordering there is: a new reply bumps an old
+ // thread to the front under newest-first. beginMoveRows, not a
+ // remove-and-insert pair, because a removed row takes its persistent
+ // index, its selection and its expansion with it, which is exactly what
+ // this method exists to keep.
+ //
+ // Always UPWARDS, and that is a property of the walk rather than an
+ // assumption about the data. Positions ahead of `target` are already
+ // final, so a survivor found at a later row is pulled forward and one
+ // found earlier cannot exist: it would have been placed on a previous
+ // iteration. A downward branch here would be unreachable, so there
+ // isn't one, and the destination needs no adjustment (Qt reads it
+ // before the source is removed, which only shifts a downward move).
+ int row = it.value();
+ if (row != target) {
+ Q_ASSERT(row > target);
+ beginMoveRows({}, row, row, {}, target);
+ m_threads.move(row, target);
+ endMoveRows();
+
+ // Every row between the two shifted one place later.
+ for (auto entry = present.begin(); entry != present.end(); ++entry) {
+ if (entry.value() >= target && entry.value() < row)
+ ++entry.value();
+ }
+ present[summary.threadId] = target;
+ row = target;
+ }
+
+ // Keep the ROW, replace the summary. The node is not reconstructed,
+ // because its children and its loaded flag are the expansion state this
+ // whole method exists to preserve.
+ if (m_threads.at(row).summary.tags != summary.tags
+ || m_threads.at(row).summary.subject != summary.subject
+ || m_threads.at(row).summary.authors != summary.authors
+ || m_threads.at(row).summary.date != summary.date
+ || m_threads.at(row).summary.totalCount != summary.totalCount
+ || m_threads.at(row).summary.matchedCount != summary.matchedCount) {
+ m_threads[row].summary = summary;
+ emit dataChanged(index(row, 0), index(row, 0));
+ }
+ }
+}
+
void ThreadListModel::clear()
{
beginResetModel();
diff --git a/src/threadlistmodel.h b/src/threadlistmodel.h
index f381ffa..01dd9d0 100644
--- a/src/threadlistmodel.h
+++ b/src/threadlistmodel.h
@@ -180,6 +180,20 @@ public:
void appendBatch(const QVector<ThreadSummary> &batch);
void clear();
+ /// Brings the model to `threads` without resetting it.
+ ///
+ /// Used by the automatic refresh after a sync, where clear() plus
+ /// appendBatch() is the wrong tool: a reset invalidates every index, so the
+ /// selection, the expanded threads and the message being read all go with
+ /// it. Rows are matched by thread id, so a surviving thread keeps its
+ /// identity, its persistent index and its loaded replies.
+ ///
+ /// Order comes from `threads` and is never imposed here. The worker sorts
+ /// the query, so a new thread lands at the front under newest-first and at
+ /// the back under oldest-first; forcing new rows to the top would
+ /// contradict the sort the user selected.
+ void reconcile(const QVector<ThreadSummary> &threads);
+
ThreadSummary threadAt(int row) const;
/// Fills in a thread's message rows once the worker has walked its tree.
diff --git a/tests/test_mainwindow.cpp b/tests/test_mainwindow.cpp
index 922705c..504c10c 100644
--- a/tests/test_mainwindow.cpp
+++ b/tests/test_mainwindow.cpp
@@ -20,6 +20,7 @@
#include <QAction>
#include <QApplication>
+#include <QFocusEvent>
#include <QCloseEvent>
#include <QDir>
#include <QKeyEvent>
@@ -99,6 +100,27 @@ private slots:
void aLocalSyncIsNotReportedAsABackgroundOne();
void aLocalSyncsOwnLockIsNeverReportedAsBackground();
void aSkippedLocalSyncStillReportsTheOtherRunFinishing();
+ void aCronSyncRefreshesTheListWithoutAQuery();
+ void aCronSyncRefreshesOverASelectionWithoutClearingIt();
+ void aCronSyncDoesNotRefreshBeforeAnyQueryHasRun();
+ void aRefreshAddsNewMailAndDropsWhatStoppedMatching();
+ void theOpenThreadLeavingTheListRaisesTheStaleNotice();
+ void aThreadStillMatchingRaisesNoStaleNotice();
+ void theStaleNoticeCarriesTheMessageBeingRead();
+ void recoveringAStaleThreadQueriesTheWholeThread();
+ void recoveryReselectsTheMessageThatWasBeingRead();
+ void recoveryOnTheFirstMessageSelectsTheThreadRow();
+ void aUserQueryAbandonsAPendingRecovery();
+ void blankingThePaneAlsoDropsTheStaleNotice();
+ void aNewQueryDropsTheStaleNotice();
+ void aFinishedBackgroundSyncStopsSayingItIsRunning();
+ void aRefreshDoesNotStampOverASelectionMessage();
+ void aRefreshDoesNotOpenNewMailByItself();
+ void openingAnotherMessageDropsTheStaleNoticeOfThePreviousOne();
+ void theStaleNoticeKeepsTheMessageOfAThreadRootToo();
+ void recoveryExpandsTheThreadAndSelectsRatherThanOnlyPointing();
+ void recoveryFromAnExpandedThreadRestoresTheReply();
+ void theRecoveryButtonSurvivesThePaneBeingBlanked();
void anUnobservableLockTableLeavesTheSyncButtonUsable();
void theStatusBarFollowsTheSyncPhase();
void aSelectedReadThreadIsNotDimmedIntoTheHighlight();
@@ -2357,12 +2379,18 @@ void TestMainWindow::aLocalSyncIsNotReportedAsABackgroundOne()
qPrintable(QStringLiteral("a background sync was not announced, "
"status says '%1'").arg(status->text())));
+ // The finish is no longer ANNOUNCED, since item 35b made the refresh
+ // silent, but it must still be acted on: the running message it wrote is
+ // retired. Asserting the absence of "running" rather than the presence of
+ // "completed" keeps the test on this window's subject, which is that the
+ // lock period was attributed to a background sync rather than to a local
+ // one, without pinning wording that has already changed once.
QMetaObject::invokeMethod(&window, "onExternalSyncStateChanged",
Q_ARG(SyncMonitor::State,
SyncMonitor::State::Idle));
- QVERIFY2(status->text().contains(QStringLiteral("Background")),
- qPrintable(QStringLiteral("a finished background sync was not "
- "announced, status says '%1'")
+ QVERIFY2(!status->text().contains(QStringLiteral("running")),
+ qPrintable(QStringLiteral("a finished background sync left the bar "
+ "claiming it was still running: '%1'")
.arg(status->text())));
}
@@ -2437,14 +2465,1029 @@ void TestMainWindow::aSkippedLocalSyncStillReportsTheOtherRunFinishing()
qPrintable(QStringLiteral("a skip was reported as a failure: '%1'")
.arg(status->text())));
- // The other run finishing must still be announced.
+ // The other run finishing must still be ACTED ON. What that means depends
+ // on the state of the list, and this fixture has an empty one with nothing
+ // selected, so item 35a's free-refresh branch is what a handed-back lock
+ // reaches: the window re-runs the query rather than printing "press Enter".
+ //
+ // Asserting the generation rather than the status text is deliberate. The
+ // property under test is that the Idle was attributed to the OTHER run
+ // instead of being swallowed as this window's own; which of the two
+ // responses it then produces is item 35a's business, and pinning the
+ // wording here would fail every time that decision is revisited.
+ auto *queryEdit = window.findChild<QLineEdit *>();
+ QVERIFY(queryEdit);
+ queryEdit->setText(QStringLiteral("tag:inbox"));
+ queryEdit->returnPressed();
+ const quint64 before = window.currentGenerationForTesting();
+
QMetaObject::invokeMethod(&window, "onExternalSyncStateChanged",
Q_ARG(SyncMonitor::State,
SyncMonitor::State::Idle));
- QVERIFY2(status->text().contains(QStringLiteral("Background")),
- qPrintable(QStringLiteral("after a skipped local sync, the other "
- "run finishing was swallowed; status "
- "says '%1'").arg(status->text())));
+ QVERIFY2(window.currentGenerationForTesting() > before,
+ "after a skipped local sync, the other run finishing was "
+ "swallowed rather than attributed to the background sync");
+}
+
+void TestMainWindow::aCronSyncRefreshesTheListWithoutAQuery()
+{
+ // Item 35b. A background sync brings the list up to date on its own, with
+ // no keystroke: this is the whole point of the item, and it holds whether
+ // the list is empty or full.
+ const Config config;
+ MainWindow window(config);
+
+ auto *queryEdit = window.findChild<QLineEdit *>();
+ QVERIFY(queryEdit);
+ queryEdit->setText(QStringLiteral("tag:unread"));
+ queryEdit->returnPressed();
+
+ // The refresh is observed as a NEW query being issued. There is no worker
+ // in this fixture, so no result ever arrives; what is asserted is that the
+ // query went out at all, which is what used to be missing.
+ const quint64 before = window.currentGenerationForTesting();
+
+ QMetaObject::invokeMethod(&window, "onExternalSyncStateChanged",
+ Q_ARG(SyncMonitor::State,
+ SyncMonitor::State::Idle));
+
+ QVERIFY2(window.currentGenerationForTesting() > before,
+ "a finished cron sync did not refresh the list");
+}
+
+void TestMainWindow::aCronSyncRefreshesOverASelectionWithoutClearingIt()
+{
+ // The behaviour 0.8.0 refused to build and the reason it refused: a refresh
+ // used to mean runCurrentQuery(), which clears the model, the selection and
+ // the message pane, so firing it on a cron timer would close the thread
+ // being read six times an hour.
+ //
+ // refreshCurrentQuery() reconciles instead, so the refresh runs AND the
+ // selection survives. A test that only checked the query was issued would
+ // pass against the destructive version, which is the version this item
+ // exists to avoid.
+ const Config config;
+ MainWindow window(config);
+
+ auto *queryEdit = window.findChild<QLineEdit *>();
+ QVERIFY(queryEdit);
+ queryEdit->setText(QStringLiteral("tag:unread"));
+ queryEdit->returnPressed();
+
+ auto *model = window.findChild<ThreadListModel *>();
+ QVERIFY(model);
+ auto *view = window.findChild<ThreadListView *>();
+ QVERIFY(view);
+
+ model->appendBatch({ makeThread(QStringLiteral("T1"),
+ { QStringLiteral("unread") }) });
+ const QModelIndex first = model->index(0, 0, QModelIndex());
+ QVERIFY(first.isValid());
+ view->setCurrentIndex(first);
+ QVERIFY2(view->selectionModel()->hasSelection(),
+ "the fixture failed to select a row, so this proves nothing");
+
+ const quint64 before = window.currentGenerationForTesting();
+
+ QMetaObject::invokeMethod(&window, "onExternalSyncStateChanged",
+ Q_ARG(SyncMonitor::State,
+ SyncMonitor::State::Idle));
+
+ QVERIFY2(window.currentGenerationForTesting() > before,
+ "a cron sync did not refresh a populated list");
+
+ // The rows are untouched until the result comes back, and the selection is
+ // still there. A clear() would have emptied both.
+ QCOMPARE(model->rowCount(QModelIndex()), 1);
+ QVERIFY2(view->selectionModel()->hasSelection(),
+ "the refresh cleared the selection, which is what made the old "
+ "one unusable on a cron timer");
+}
+
+void TestMainWindow::aCronSyncDoesNotRefreshBeforeAnyQueryHasRun()
+{
+ // The query bar holds text the user has typed but not run, and a refresh
+ // must not execute it: that is a search they never asked for. The refresh
+ // re-runs the LAST RUN query, so with none there is nothing to do.
+ const Config config;
+ MainWindow window(config);
+
+ auto *queryEdit = window.findChild<QLineEdit *>();
+ QVERIFY(queryEdit);
+ queryEdit->setText(QStringLiteral("tag:draft-i-was-typing"));
+
+ const quint64 before = window.currentGenerationForTesting();
+
+ QMetaObject::invokeMethod(&window, "onExternalSyncStateChanged",
+ Q_ARG(SyncMonitor::State,
+ SyncMonitor::State::Idle));
+
+ QCOMPARE(window.currentGenerationForTesting(), before);
+}
+
+void TestMainWindow::aRefreshAddsNewMailAndDropsWhatStoppedMatching()
+{
+ // The round trip end to end, driven through the real handlers: the refresh
+ // query goes out, its batches accumulate, and the result reconciles into
+ // the model in one go.
+ const Config config;
+ MainWindow window(config);
+
+ auto *queryEdit = window.findChild<QLineEdit *>();
+ QVERIFY(queryEdit);
+ queryEdit->setText(QStringLiteral("tag:unread"));
+ queryEdit->returnPressed();
+
+ auto *model = window.findChild<ThreadListModel *>();
+ QVERIFY(model);
+ model->appendBatch({ makeThread(QStringLiteral("T1"),
+ { QStringLiteral("unread") }),
+ makeThread(QStringLiteral("T2"),
+ { QStringLiteral("unread") }) });
+ QCOMPARE(model->rowCount(QModelIndex()), 2);
+
+ QMetaObject::invokeMethod(&window, "onExternalSyncStateChanged",
+ Q_ARG(SyncMonitor::State,
+ SyncMonitor::State::Idle));
+ const quint64 refresh = window.currentGenerationForTesting();
+
+ // T2 was read elsewhere and no longer matches; T3 is new mail.
+ const QVector<ThreadSummary> result{
+ makeThread(QStringLiteral("T3"), { QStringLiteral("unread") }),
+ makeThread(QStringLiteral("T1"), { QStringLiteral("unread") })
+ };
+ QMetaObject::invokeMethod(&window, "onThreadsReady",
+ Q_ARG(QVector<ThreadSummary>, result),
+ Q_ARG(quint64, refresh));
+
+ // Nothing has changed yet: a refresh applies its result whole, never batch
+ // by batch, or the first batch would delete every row after it.
+ QCOMPARE(model->rowCount(QModelIndex()), 2);
+ QCOMPARE(model->threadAt(0).threadId, QStringLiteral("T1"));
+
+ QMetaObject::invokeMethod(&window, "onQueryFinished",
+ Q_ARG(int, 2), Q_ARG(quint64, refresh));
+
+ QCOMPARE(model->rowCount(QModelIndex()), 2);
+ QCOMPARE(model->threadAt(0).threadId, QStringLiteral("T3"));
+ QCOMPARE(model->threadAt(1).threadId, QStringLiteral("T1"));
+}
+
+void TestMainWindow::theOpenThreadLeavingTheListRaisesTheStaleNotice()
+{
+ // The user is reading a thread when a refresh drops it: read the last
+ // unread message and the thread stops matching tag:unread. The pane keeps
+ // rendering it, correctly, so without a notice the message becomes an
+ // orphan with no route back to its thread.
+ const Config config;
+ MainWindow window(config);
+
+ auto *queryEdit = window.findChild<QLineEdit *>();
+ QVERIFY(queryEdit);
+ queryEdit->setText(QStringLiteral("tag:unread"));
+ queryEdit->returnPressed();
+
+ auto *model = window.findChild<ThreadListModel *>();
+ QVERIFY(model);
+ auto *view = window.findChild<ThreadListView *>();
+ QVERIFY(view);
+ auto *pane = window.findChild<MessageView *>();
+ QVERIFY(pane);
+
+ model->appendBatch({ makeThread(QStringLiteral("T1"),
+ { QStringLiteral("unread") }) });
+ view->setCurrentIndex(model->index(0, 0, QModelIndex()));
+ QCOMPARE(window.currentThreadId(), QStringLiteral("T1"));
+ QVERIFY(pane->staleThreadId().isEmpty());
+
+ QMetaObject::invokeMethod(&window, "onExternalSyncStateChanged",
+ Q_ARG(SyncMonitor::State,
+ SyncMonitor::State::Idle));
+ const quint64 refresh = window.currentGenerationForTesting();
+
+ // The refresh comes back empty: the thread was read and is gone.
+ QMetaObject::invokeMethod(&window, "onQueryFinished",
+ Q_ARG(int, 0), Q_ARG(quint64, refresh));
+
+ QCOMPARE(model->rowCount(QModelIndex()), 0);
+ QCOMPARE(pane->staleThreadId(), QStringLiteral("T1"));
+}
+
+void TestMainWindow::aThreadStillMatchingRaisesNoStaleNotice()
+{
+ // The common case, and the guard on the test above: a refresh that changes
+ // nothing must be invisible. A notice on every sync would be noise, and it
+ // would be a lie.
+ const Config config;
+ MainWindow window(config);
+
+ auto *queryEdit = window.findChild<QLineEdit *>();
+ QVERIFY(queryEdit);
+ queryEdit->setText(QStringLiteral("tag:unread"));
+ queryEdit->returnPressed();
+
+ auto *model = window.findChild<ThreadListModel *>();
+ QVERIFY(model);
+ auto *view = window.findChild<ThreadListView *>();
+ QVERIFY(view);
+ auto *pane = window.findChild<MessageView *>();
+ QVERIFY(pane);
+
+ model->appendBatch({ makeThread(QStringLiteral("T1"),
+ { QStringLiteral("unread") }) });
+ view->setCurrentIndex(model->index(0, 0, QModelIndex()));
+
+ QMetaObject::invokeMethod(&window, "onExternalSyncStateChanged",
+ Q_ARG(SyncMonitor::State,
+ SyncMonitor::State::Idle));
+ const quint64 refresh = window.currentGenerationForTesting();
+
+ const QVector<ThreadSummary> result{
+ makeThread(QStringLiteral("T1"), { QStringLiteral("unread") })
+ };
+ QMetaObject::invokeMethod(&window, "onThreadsReady",
+ Q_ARG(QVector<ThreadSummary>, result),
+ Q_ARG(quint64, refresh));
+ QMetaObject::invokeMethod(&window, "onQueryFinished",
+ Q_ARG(int, 1), Q_ARG(quint64, refresh));
+
+ QCOMPARE(model->rowCount(QModelIndex()), 1);
+ QVERIFY2(pane->staleThreadId().isEmpty(),
+ "a thread that still matches was reported as stale");
+}
+
+void TestMainWindow::theStaleNoticeCarriesTheMessageBeingRead()
+{
+ // Reading reply four of eight when the thread drops out. Recovery has to
+ // restore the READER'S place, so the notice carries the message id as well
+ // as the thread; without it the thread reopens at its first message.
+ //
+ // Selecting a message row clears m_currentThreadId, so a notice keyed on
+ // that alone never fires for exactly the reader who is deepest into a
+ // thread. That is the case this pins.
+ const Config config;
+ MainWindow window(config);
+
+ auto *queryEdit = window.findChild<QLineEdit *>();
+ QVERIFY(queryEdit);
+ queryEdit->setText(QStringLiteral("tag:unread"));
+ queryEdit->returnPressed();
+
+ auto *model = window.findChild<ThreadListModel *>();
+ QVERIFY(model);
+ auto *view = window.findChild<ThreadListView *>();
+ QVERIFY(view);
+ auto *pane = window.findChild<MessageView *>();
+ QVERIFY(pane);
+
+ ThreadSummary thread = makeThread(QStringLiteral("T1"),
+ { QStringLiteral("unread") });
+ thread.totalCount = 3;
+ model->appendBatch({ thread });
+
+ MessageNode root;
+ root.messageId = QStringLiteral("m0@example.org");
+ root.threadId = QStringLiteral("T1");
+ root.depth = 0;
+ MessageNode reply;
+ reply.messageId = QStringLiteral("m1@example.org");
+ reply.threadId = QStringLiteral("T1");
+ reply.depth = 1;
+ model->setThreadMessages(QStringLiteral("T1"), { root, reply });
+
+ const QModelIndex threadIndex = model->index(0, 0, QModelIndex());
+ const QModelIndex replyIndex = model->index(0, 0, threadIndex);
+ QVERIFY(replyIndex.isValid());
+ view->setCurrentIndex(replyIndex);
+
+ // A message row, so the window is tracking a message rather than a thread.
+ QVERIFY2(window.currentThreadId().isEmpty(),
+ "the fixture selected a thread row, so this proves nothing");
+
+ QMetaObject::invokeMethod(&window, "onExternalSyncStateChanged",
+ Q_ARG(SyncMonitor::State,
+ SyncMonitor::State::Idle));
+ const quint64 refresh = window.currentGenerationForTesting();
+ QMetaObject::invokeMethod(&window, "onQueryFinished",
+ Q_ARG(int, 0), Q_ARG(quint64, refresh));
+
+ QCOMPARE(pane->staleThreadId(), QStringLiteral("T1"));
+ QCOMPARE(pane->staleMessageId(), QStringLiteral("m1@example.org"));
+}
+
+void TestMainWindow::recoveringAStaleThreadQueriesTheWholeThread()
+{
+ // Clicking "Show it anyway" runs thread:<id>, not a query for the single
+ // message: the user asked to get the whole conversation back, with their
+ // place in it, so the list has to offer every message of it.
+ const Config config;
+ MainWindow window(config);
+
+ auto *queryEdit = window.findChild<QLineEdit *>();
+ QVERIFY(queryEdit);
+ queryEdit->setText(QStringLiteral("tag:unread"));
+ queryEdit->returnPressed();
+
+ QMetaObject::invokeMethod(&window, "recoverStaleThread",
+ Q_ARG(QString, QStringLiteral("T1")),
+ Q_ARG(QString, QStringLiteral("m1@example.org")));
+
+ QCOMPARE(queryEdit->text(), QStringLiteral("thread:T1"));
+}
+
+void TestMainWindow::recoveryReselectsTheMessageThatWasBeingRead()
+{
+ // The whole point of carrying the message id: reading reply four of eight,
+ // the thread comes back, and the selection lands on reply four rather than
+ // at the top of the thread.
+ //
+ // Driven through the real handlers because that is the only way the
+ // sequencing is exercised: the query has to come back before the thread
+ // can be expanded, and the expansion before the reply row exists at all.
+ const Config config;
+ MainWindow window(config);
+
+ auto *queryEdit = window.findChild<QLineEdit *>();
+ QVERIFY(queryEdit);
+ queryEdit->setText(QStringLiteral("tag:unread"));
+ queryEdit->returnPressed();
+
+ auto *model = window.findChild<ThreadListModel *>();
+ QVERIFY(model);
+ auto *view = window.findChild<ThreadListView *>();
+ QVERIFY(view);
+
+ QMetaObject::invokeMethod(&window, "recoverStaleThread",
+ Q_ARG(QString, QStringLiteral("T1")),
+ Q_ARG(QString, QStringLiteral("m2@example.org")));
+ const quint64 generation = window.currentGenerationForTesting();
+
+ ThreadSummary thread = makeThread(QStringLiteral("T1"), {});
+ thread.totalCount = 3;
+ const QVector<ThreadSummary> result{ thread };
+ QMetaObject::invokeMethod(&window, "onThreadsReady",
+ Q_ARG(QVector<ThreadSummary>, result),
+ Q_ARG(quint64, generation));
+ QMetaObject::invokeMethod(&window, "onQueryFinished",
+ Q_ARG(int, 1), Q_ARG(quint64, generation));
+
+ // The thread is listed, and nothing can be selected inside it yet: its
+ // replies are not loaded, so the recovery is still pending.
+ QCOMPARE(model->rowCount(QModelIndex()), 1);
+
+ MessageNode root;
+ root.messageId = QStringLiteral("m0@example.org");
+ root.threadId = QStringLiteral("T1");
+ root.depth = 0;
+ MessageNode first;
+ first.messageId = QStringLiteral("m1@example.org");
+ first.threadId = QStringLiteral("T1");
+ first.depth = 1;
+ MessageNode target;
+ target.messageId = QStringLiteral("m2@example.org");
+ target.threadId = QStringLiteral("T1");
+ target.depth = 1;
+ const QVector<MessageNode> nodes{ root, first, target };
+ QMetaObject::invokeMethod(&window, "onThreadTreeLoaded",
+ Q_ARG(QVector<MessageNode>, nodes),
+ Q_ARG(quint64, generation));
+
+ const QModelIndex current = view->currentIndex();
+ QVERIFY2(current.isValid(), "recovery selected nothing");
+ QVERIFY2(model->isMessageRow(current),
+ "recovery landed on the thread rather than on the message");
+ QCOMPARE(model->messageAt(current).messageId,
+ QStringLiteral("m2@example.org"));
+}
+
+void TestMainWindow::recoveryOnTheFirstMessageSelectsTheThreadRow()
+{
+ // The trap in the model: setThreadMessages DROPS the depth-0 message,
+ // because the root card is that message. So a reader recovering from the
+ // thread's first message must land on the ROOT row; looking for it among
+ // the children finds nothing and would leave the selection nowhere.
+ const Config config;
+ MainWindow window(config);
+
+ auto *queryEdit = window.findChild<QLineEdit *>();
+ QVERIFY(queryEdit);
+ queryEdit->setText(QStringLiteral("tag:unread"));
+ queryEdit->returnPressed();
+
+ auto *model = window.findChild<ThreadListModel *>();
+ QVERIFY(model);
+ auto *view = window.findChild<ThreadListView *>();
+ QVERIFY(view);
+
+ QMetaObject::invokeMethod(&window, "recoverStaleThread",
+ Q_ARG(QString, QStringLiteral("T1")),
+ Q_ARG(QString, QStringLiteral("m0@example.org")));
+ const quint64 generation = window.currentGenerationForTesting();
+
+ ThreadSummary thread = makeThread(QStringLiteral("T1"), {});
+ thread.totalCount = 2;
+ const QVector<ThreadSummary> result{ thread };
+ QMetaObject::invokeMethod(&window, "onThreadsReady",
+ Q_ARG(QVector<ThreadSummary>, result),
+ Q_ARG(quint64, generation));
+
+ MessageNode root;
+ root.messageId = QStringLiteral("m0@example.org");
+ root.threadId = QStringLiteral("T1");
+ root.depth = 0;
+ MessageNode reply;
+ reply.messageId = QStringLiteral("m1@example.org");
+ reply.threadId = QStringLiteral("T1");
+ reply.depth = 1;
+ const QVector<MessageNode> nodes{ root, reply };
+ QMetaObject::invokeMethod(&window, "onThreadTreeLoaded",
+ Q_ARG(QVector<MessageNode>, nodes),
+ Q_ARG(quint64, generation));
+
+ QMetaObject::invokeMethod(&window, "onQueryFinished",
+ Q_ARG(int, 1), Q_ARG(quint64, generation));
+
+ const QModelIndex current = view->currentIndex();
+ QVERIFY2(current.isValid(), "recovery selected nothing");
+ QVERIFY2(!model->isMessageRow(current),
+ "the thread's first message is the ROOT row, not a child");
+ QCOMPARE(model->threadAt(current.row()).threadId, QStringLiteral("T1"));
+}
+
+void TestMainWindow::aUserQueryAbandonsAPendingRecovery()
+{
+ // A recovery spans two round-trips, so the user can type a query in the
+ // middle of one. That is them choosing to go somewhere else, and the
+ // pending selection must not follow them there: restoring a thread's
+ // message into a result the user asked for something else from would yank
+ // the view out from under them.
+ const Config config;
+ MainWindow window(config);
+
+ auto *queryEdit = window.findChild<QLineEdit *>();
+ QVERIFY(queryEdit);
+ auto *model = window.findChild<ThreadListModel *>();
+ QVERIFY(model);
+ auto *view = window.findChild<ThreadListView *>();
+ QVERIFY(view);
+
+ // Recovering the thread with NO message pinned, so the pending recovery
+ // selects its thread row the moment that thread appears. A recovery
+ // waiting on a specific reply would pass this test without the guard,
+ // simply by never reaching its target: it would sit expanding a thread
+ // whose replies this fixture never delivers.
+ QMetaObject::invokeMethod(&window, "recoverStaleThread",
+ Q_ARG(QString, QStringLiteral("T1")),
+ Q_ARG(QString, QString()));
+
+ // The user changes their mind before the recovery's query comes back.
+ queryEdit->setText(QStringLiteral("tag:flagged"));
+ queryEdit->returnPressed();
+ const quint64 generation = window.currentGenerationForTesting();
+
+ // That query happens to contain the same thread, which is what makes this
+ // a trap rather than a theoretical case: the recovery would find its
+ // target and select it.
+ ThreadSummary thread = makeThread(QStringLiteral("T1"), {});
+ thread.totalCount = 2;
+ const QVector<ThreadSummary> result{
+ makeThread(QStringLiteral("T9"), {}), thread
+ };
+ QMetaObject::invokeMethod(&window, "onThreadsReady",
+ Q_ARG(QVector<ThreadSummary>, result),
+ Q_ARG(quint64, generation));
+ QMetaObject::invokeMethod(&window, "onQueryFinished",
+ Q_ARG(int, 2), Q_ARG(quint64, generation));
+
+ QVERIFY2(!view->currentIndex().isValid(),
+ "an abandoned recovery selected a row in the query the user ran "
+ "instead");
+}
+
+void TestMainWindow::blankingThePaneAlsoDropsTheStaleNotice()
+{
+ // Reported by the user against the first build of item 35b. The notice
+ // outlived the message it describes: blanking the pane left the bar sitting
+ // above an empty pane, still naming a thread that was no longer shown, with
+ // a button offering to recover it.
+ //
+ // The bar belongs to the rendered message, exactly as the remote-content
+ // bar does, and MessageView::clear() already hides that one. This is the
+ // same rule applied to the same place.
+ const Config config;
+ MainWindow window(config);
+
+ auto *pane = window.findChild<MessageView *>();
+ QVERIFY(pane);
+
+ pane->setStaleThread(QStringLiteral("T1"),
+ QStringLiteral("m1@example.org"));
+ QCOMPARE(pane->staleThreadId(), QStringLiteral("T1"));
+
+ pane->clear();
+
+ QVERIFY2(pane->staleThreadId().isEmpty(),
+ "the stale notice survived the pane being blanked, so it names a "
+ "message that is no longer displayed");
+ QVERIFY2(pane->staleMessageId().isEmpty(),
+ "the stale notice kept the message id of a cleared pane");
+}
+
+void TestMainWindow::aNewQueryDropsTheStaleNotice()
+{
+ // The user's actual route to the bug: read a thread out of a view, get the
+ // notice, then type a new query. The pane blanks and the bar must go with
+ // it. Driven through the window rather than through MessageView::clear()
+ // directly, because the defect was that nothing on this path called it.
+ const Config config;
+ MainWindow window(config);
+
+ auto *queryEdit = window.findChild<QLineEdit *>();
+ QVERIFY(queryEdit);
+ auto *model = window.findChild<ThreadListModel *>();
+ QVERIFY(model);
+ auto *view = window.findChild<ThreadListView *>();
+ QVERIFY(view);
+ auto *pane = window.findChild<MessageView *>();
+ QVERIFY(pane);
+
+ queryEdit->setText(QStringLiteral("tag:inbox"));
+ queryEdit->returnPressed();
+
+ model->appendBatch({ makeThread(QStringLiteral("T1"),
+ { QStringLiteral("unread") }) });
+ view->setCurrentIndex(model->index(0, 0, QModelIndex()));
+
+ QMetaObject::invokeMethod(&window, "onExternalSyncStateChanged",
+ Q_ARG(SyncMonitor::State,
+ SyncMonitor::State::Idle));
+ const quint64 refresh = window.currentGenerationForTesting();
+ QMetaObject::invokeMethod(&window, "onQueryFinished",
+ Q_ARG(int, 0), Q_ARG(quint64, refresh));
+ QCOMPARE(pane->staleThreadId(), QStringLiteral("T1"));
+
+ // The user runs a different query.
+ queryEdit->setText(QStringLiteral("tag:unread"));
+ queryEdit->returnPressed();
+
+ QVERIFY2(pane->staleThreadId().isEmpty(),
+ "after a new query the notice still offered to recover the thread "
+ "from the previous view");
+}
+
+void TestMainWindow::aFinishedBackgroundSyncStopsSayingItIsRunning()
+{
+ // Reported by the user against item 35b. "Background sync running..." is
+ // written straight to the status label when the lock appears, and the
+ // "Background sync completed" message on the way out was the only thing
+ // that ever replaced it. Removing that message, so a refresh could be
+ // silent, left the bar claiming a sync was running long after it finished.
+ //
+ // Silent means "says nothing NEW", not "leaves a stale claim standing".
+ const Config config;
+ MainWindow window(config);
+
+ auto *status = window.findChild<QLabel *>(QStringLiteral("statusMessage"));
+ QVERIFY(status);
+
+ QMetaObject::invokeMethod(&window, "onExternalSyncStateChanged",
+ Q_ARG(SyncMonitor::State,
+ SyncMonitor::State::Running));
+ QVERIFY2(status->text().contains(QStringLiteral("running")),
+ qPrintable(QStringLiteral("the fixture never announced a running "
+ "sync, status says '%1'")
+ .arg(status->text())));
+
+ QMetaObject::invokeMethod(&window, "onExternalSyncStateChanged",
+ Q_ARG(SyncMonitor::State,
+ SyncMonitor::State::Idle));
+
+ QVERIFY2(!status->text().contains(QStringLiteral("running")),
+ qPrintable(QStringLiteral("the status bar still claims a sync is "
+ "running after it finished: '%1'")
+ .arg(status->text())));
+}
+
+void TestMainWindow::aRefreshDoesNotStampOverASelectionMessage()
+{
+ // The other half, and the reason this is not simply "always write the
+ // thread count". A refresh runs on a cron timer under a user who may be
+ // doing something, and the bar carries their selection count while they
+ // are. Overwriting that every ten minutes is the noise the silence rule
+ // exists to prevent.
+ const Config config;
+ MainWindow window(config);
+
+ auto *status = window.findChild<QLabel *>(QStringLiteral("statusMessage"));
+ QVERIFY(status);
+ auto *queryEdit = window.findChild<QLineEdit *>();
+ QVERIFY(queryEdit);
+ auto *model = window.findChild<ThreadListModel *>();
+ QVERIFY(model);
+ auto *view = window.findChild<ThreadListView *>();
+ QVERIFY(view);
+
+ queryEdit->setText(QStringLiteral("tag:inbox"));
+ queryEdit->returnPressed();
+
+ model->appendBatch({ makeThread(QStringLiteral("T1"), {}),
+ makeThread(QStringLiteral("T2"), {}) });
+ view->setCurrentIndex(model->index(0, 0, QModelIndex()));
+ view->selectionModel()->select(model->index(1, 0, QModelIndex()),
+ QItemSelectionModel::Select);
+
+ const QString before = status->text();
+ QVERIFY2(!before.isEmpty(),
+ "the fixture left the status bar empty, so this proves nothing");
+
+ // A refresh completes with no sync ever having been announced.
+ QMetaObject::invokeMethod(&window, "onExternalSyncStateChanged",
+ Q_ARG(SyncMonitor::State,
+ SyncMonitor::State::Idle));
+ const quint64 refresh = window.currentGenerationForTesting();
+ const QVector<ThreadSummary> result{
+ makeThread(QStringLiteral("T1"), {}), makeThread(QStringLiteral("T2"), {})
+ };
+ QMetaObject::invokeMethod(&window, "onThreadsReady",
+ Q_ARG(QVector<ThreadSummary>, result),
+ Q_ARG(quint64, refresh));
+ QMetaObject::invokeMethod(&window, "onQueryFinished",
+ Q_ARG(int, 2), Q_ARG(quint64, refresh));
+
+ QCOMPARE(status->text(), before);
+}
+
+void TestMainWindow::aRefreshDoesNotOpenNewMailByItself()
+{
+ // Reported by the user against item 35b. A thread read to the end empties
+ // an Unread view; the refresh then brings in one new message, and it opens
+ // ITSELF in the message pane, marking it read two seconds later without
+ // the user ever having looked at it.
+ //
+ // Nothing in MainWindow selects it: QTreeView sets a current index of its
+ // own when rows are inserted into a model that had none, and selecting a
+ // row is what loads it. An automatic refresh must not do that, or a cron
+ // timer decides what the user is reading.
+ const Config config;
+ MainWindow window(config);
+
+ auto *queryEdit = window.findChild<QLineEdit *>();
+ QVERIFY(queryEdit);
+ auto *model = window.findChild<ThreadListModel *>();
+ QVERIFY(model);
+ auto *view = window.findChild<ThreadListView *>();
+ QVERIFY(view);
+
+ queryEdit->setText(QStringLiteral("tag:unread"));
+ queryEdit->returnPressed();
+
+ // The view has been read empty: no rows, nothing current.
+ QCOMPARE(model->rowCount(QModelIndex()), 0);
+ QVERIFY(!view->currentIndex().isValid());
+
+ QMetaObject::invokeMethod(&window, "onExternalSyncStateChanged",
+ Q_ARG(SyncMonitor::State,
+ SyncMonitor::State::Idle));
+ const quint64 refresh = window.currentGenerationForTesting();
+
+ const QVector<ThreadSummary> result{
+ makeThread(QStringLiteral("NEW"), { QStringLiteral("unread") })
+ };
+ QMetaObject::invokeMethod(&window, "onThreadsReady",
+ Q_ARG(QVector<ThreadSummary>, result),
+ Q_ARG(quint64, refresh));
+ QMetaObject::invokeMethod(&window, "onQueryFinished",
+ Q_ARG(int, 1), Q_ARG(quint64, refresh));
+
+ QCOMPARE(model->rowCount(QModelIndex()), 1);
+
+ // Insertion alone does not do it, which is why the bug only showed up when
+ // the user came back from another desktop: QTreeView gives itself a current
+ // index when it takes FOCUS with none set. Reproduced here rather than
+ // asserted from the report, since a test that only inserts rows passes
+ // against the defect.
+ view->setFocus();
+ QApplication::sendEvent(view, new QFocusEvent(QEvent::FocusIn));
+
+ QVERIFY2(window.currentThreadId().isEmpty(),
+ "the refresh opened the new mail in the message pane, which marks "
+ "it read without the user having looked at it");
+}
+
+void TestMainWindow::openingAnotherMessageDropsTheStaleNoticeOfThePreviousOne()
+{
+ // The other half of the user's report: the pane showed the new message
+ // while the notice above it still named the thread they had been reading.
+ //
+ // The notice itself was right at the moment it was raised. What made it a
+ // lie was the pane being replaced underneath it, so this pins the rule that
+ // the notice belongs to whatever is currently rendered: selecting anything
+ // else retires it, exactly as blanking the pane does.
+ const Config config;
+ MainWindow window(config);
+
+ auto *queryEdit = window.findChild<QLineEdit *>();
+ QVERIFY(queryEdit);
+ auto *model = window.findChild<ThreadListModel *>();
+ QVERIFY(model);
+ auto *view = window.findChild<ThreadListView *>();
+ QVERIFY(view);
+ auto *pane = window.findChild<MessageView *>();
+ QVERIFY(pane);
+
+ queryEdit->setText(QStringLiteral("tag:unread"));
+ queryEdit->returnPressed();
+
+ model->appendBatch({ makeThread(QStringLiteral("OLD"),
+ { QStringLiteral("unread") }) });
+ view->setCurrentIndex(model->index(0, 0, QModelIndex()));
+
+ // The refresh drops the thread being read and brings in new mail.
+ QMetaObject::invokeMethod(&window, "onExternalSyncStateChanged",
+ Q_ARG(SyncMonitor::State,
+ SyncMonitor::State::Idle));
+ const quint64 refresh = window.currentGenerationForTesting();
+ const QVector<ThreadSummary> result{
+ makeThread(QStringLiteral("NEW"), { QStringLiteral("unread") })
+ };
+ QMetaObject::invokeMethod(&window, "onThreadsReady",
+ Q_ARG(QVector<ThreadSummary>, result),
+ Q_ARG(quint64, refresh));
+ QMetaObject::invokeMethod(&window, "onQueryFinished",
+ Q_ARG(int, 1), Q_ARG(quint64, refresh));
+
+ QCOMPARE(pane->staleThreadId(), QStringLiteral("OLD"));
+
+ // The user chooses to open the new mail themselves.
+ const QModelIndex fresh = model->index(0, 0, QModelIndex());
+ QVERIFY(fresh.isValid());
+ QCOMPARE(model->threadAt(0).threadId, QStringLiteral("NEW"));
+ view->setCurrentIndex(fresh);
+ view->selectionModel()->select(fresh, QItemSelectionModel::Select);
+
+ QVERIFY2(pane->staleThreadId().isEmpty(),
+ "the notice still named the previous thread while the pane showed "
+ "a different message");
+}
+
+void TestMainWindow::theStaleNoticeKeepsTheMessageOfAThreadRootToo()
+{
+ // Reported by the user: recovering a thread they were reading brought the
+ // thread back collapsed, with the pane blank, instead of reopening the
+ // message they had been on.
+ //
+ // The cause is that a thread ROOT sets both ids. The root card IS the
+ // thread's first message and the pane renders exactly that message, so
+ // m_currentThreadId and m_currentMessageId are both filled; the notice read
+ // the message id only when the thread id was EMPTY, so the root case threw
+ // away a message id it had. Recovery then had nothing to restore, landed on
+ // the thread row and never expanded it.
+ const Config config;
+ MainWindow window(config);
+
+ auto *queryEdit = window.findChild<QLineEdit *>();
+ QVERIFY(queryEdit);
+ auto *model = window.findChild<ThreadListModel *>();
+ QVERIFY(model);
+ auto *view = window.findChild<ThreadListView *>();
+ QVERIFY(view);
+ auto *pane = window.findChild<MessageView *>();
+ QVERIFY(pane);
+
+ queryEdit->setText(QStringLiteral("tag:unread"));
+ queryEdit->returnPressed();
+
+ ThreadSummary thread = makeThread(QStringLiteral("T1"),
+ { QStringLiteral("unread") });
+ thread.totalCount = 4;
+ model->appendBatch({ thread });
+
+ // The root knows its own message once the tree is loaded, which is what
+ // makes the pane show one message rather than the conversation.
+ MessageNode root;
+ root.messageId = QStringLiteral("m0@example.org");
+ root.threadId = QStringLiteral("T1");
+ root.depth = 0;
+ MessageNode reply;
+ reply.messageId = QStringLiteral("m1@example.org");
+ reply.threadId = QStringLiteral("T1");
+ reply.depth = 1;
+ model->setThreadMessages(QStringLiteral("T1"), { root, reply });
+
+ const QModelIndex threadIndex = model->index(0, 0, QModelIndex());
+ view->setCurrentIndex(threadIndex);
+ view->selectionModel()->select(threadIndex, QItemSelectionModel::Select);
+ QCOMPARE(window.currentThreadId(), QStringLiteral("T1"));
+
+ QMetaObject::invokeMethod(&window, "onExternalSyncStateChanged",
+ Q_ARG(SyncMonitor::State,
+ SyncMonitor::State::Idle));
+ const quint64 refresh = window.currentGenerationForTesting();
+ QMetaObject::invokeMethod(&window, "onQueryFinished",
+ Q_ARG(int, 0), Q_ARG(quint64, refresh));
+
+ QCOMPARE(pane->staleThreadId(), QStringLiteral("T1"));
+ QVERIFY2(!pane->staleMessageId().isEmpty(),
+ "the notice dropped the message of a thread root, so recovery has "
+ "nothing to reopen and lands on a collapsed thread");
+ QCOMPARE(pane->staleMessageId(), QStringLiteral("m0@example.org"));
+}
+
+void TestMainWindow::recoveryExpandsTheThreadAndSelectsRatherThanOnlyPointing()
+{
+ // The rest of the same report: recovery brought the thread back COLLAPSED
+ // with the pane BLANK. Two separate faults behind one symptom.
+ //
+ // setCurrentIndex() alone sets a current row without selecting it, and
+ // since the fix for the auto-open defect onThreadSelected() ignores exactly
+ // that: an unselected current index is Qt's housekeeping, not the user. So
+ // recovery pointed at the row and nothing rendered.
+ //
+ // And nothing expanded the thread, so the reply the user had been reading
+ // was not on screen even when it was the target.
+ const Config config;
+ MainWindow window(config);
+
+ auto *queryEdit = window.findChild<QLineEdit *>();
+ QVERIFY(queryEdit);
+ auto *model = window.findChild<ThreadListModel *>();
+ QVERIFY(model);
+ auto *view = window.findChild<ThreadListView *>();
+ QVERIFY(view);
+
+ queryEdit->setText(QStringLiteral("tag:unread"));
+ queryEdit->returnPressed();
+
+ // Recovering onto the thread's FIRST message, which is the root card: the
+ // case the user hit by opening a thread rather than a reply.
+ QMetaObject::invokeMethod(&window, "recoverStaleThread",
+ Q_ARG(QString, QStringLiteral("T1")),
+ Q_ARG(QString, QStringLiteral("m0@example.org")));
+ const quint64 generation = window.currentGenerationForTesting();
+
+ ThreadSummary thread = makeThread(QStringLiteral("T1"), {});
+ thread.totalCount = 4;
+ const QVector<ThreadSummary> result{ thread };
+ QMetaObject::invokeMethod(&window, "onThreadsReady",
+ Q_ARG(QVector<ThreadSummary>, result),
+ Q_ARG(quint64, generation));
+ QMetaObject::invokeMethod(&window, "onQueryFinished",
+ Q_ARG(int, 1), Q_ARG(quint64, generation));
+
+ // No tree reply. This is the user's actual case and the one the earlier
+ // test missed: the thread comes back from the query with its replies NOT
+ // yet loaded, which is the normal state of a freshly queried row. Recovery
+ // has to ask for them rather than assuming they are already there.
+ const QModelIndex threadIndex = model->index(0, 0, QModelIndex());
+ QVERIFY(threadIndex.isValid());
+ QCOMPARE(model->rowCount(threadIndex), 0);
+
+ QVERIFY2(view->selectionModel()->hasSelection(),
+ "recovery pointed at the row without selecting it, so nothing "
+ "renders and the pane stays blank");
+ QCOMPARE(window.currentThreadId(), QStringLiteral("T1"));
+ QVERIFY2(view->isExpanded(threadIndex),
+ "recovery brought the thread back collapsed, so the conversation "
+ "the user was reading is not on screen");
+}
+
+void TestMainWindow::recoveryFromAnExpandedThreadRestoresTheReply()
+{
+ // The user's case, staged exactly: a thread ALREADY EXPANDED with the
+ // fourth reply selected and rendered, dropped by a refresh, then recovered.
+ // Reported twice as still broken while the earlier recovery tests passed,
+ // which means those tests were not reproducing it.
+ //
+ // What they missed is the whole round trip. Recovery re-runs thread:<id>,
+ // and that query REPLACES the model contents, so the recovered thread
+ // arrives collapsed with no replies loaded whatever state the old row was
+ // in. The reply the user wants therefore does not exist as a row at the
+ // moment recovery first runs, and the only thing that can create it is the
+ // tree reply arriving after an expand.
+ const Config config;
+ MainWindow window(config);
+
+ auto *queryEdit = window.findChild<QLineEdit *>();
+ QVERIFY(queryEdit);
+ auto *model = window.findChild<ThreadListModel *>();
+ QVERIFY(model);
+ auto *view = window.findChild<ThreadListView *>();
+ QVERIFY(view);
+
+ queryEdit->setText(QStringLiteral("tag:unread"));
+ queryEdit->returnPressed();
+
+ // Reading reply 4 of a 4-message thread, expanded.
+ ThreadSummary thread = makeThread(QStringLiteral("T1"),
+ { QStringLiteral("unread") });
+ thread.totalCount = 4;
+ model->appendBatch({ thread });
+
+ QVector<MessageNode> nodes;
+ for (int i = 0; i < 4; ++i) {
+ MessageNode n;
+ n.messageId = QStringLiteral("m%1@example.org").arg(i);
+ n.threadId = QStringLiteral("T1");
+ n.depth = i == 0 ? 0 : 1;
+ nodes.append(n);
+ }
+ model->setThreadMessages(QStringLiteral("T1"), nodes);
+
+ const QModelIndex threadIndex = model->index(0, 0, QModelIndex());
+ view->expand(threadIndex);
+ QCOMPARE(model->rowCount(threadIndex), 3);
+
+ const QModelIndex fourth = model->index(2, 0, threadIndex);
+ QVERIFY(fourth.isValid());
+ QCOMPARE(model->messageAt(fourth).messageId,
+ QStringLiteral("m3@example.org"));
+ view->setCurrentIndex(fourth);
+ view->selectionModel()->select(fourth, QItemSelectionModel::Select);
+
+ // The refresh drops it, and the user follows the notice.
+ QMetaObject::invokeMethod(&window, "recoverStaleThread",
+ Q_ARG(QString, QStringLiteral("T1")),
+ Q_ARG(QString, QStringLiteral("m3@example.org")));
+ const quint64 generation = window.currentGenerationForTesting();
+
+ // The recovery query comes back: ONE collapsed thread, no replies. This is
+ // what the query really returns, and the state the earlier tests skipped.
+ const QVector<ThreadSummary> recovered{ thread };
+ QMetaObject::invokeMethod(&window, "onThreadsReady",
+ Q_ARG(QVector<ThreadSummary>, recovered),
+ Q_ARG(quint64, generation));
+ QMetaObject::invokeMethod(&window, "onQueryFinished",
+ Q_ARG(int, 1), Q_ARG(quint64, generation));
+
+ const QModelIndex back = model->index(0, 0, QModelIndex());
+ QVERIFY(back.isValid());
+ QVERIFY2(view->isExpanded(back),
+ "the recovered thread came back collapsed");
+
+ // Expanding asks the worker for the tree; that reply is what creates the
+ // reply rows. Without it the conversation is not on screen at all.
+ QMetaObject::invokeMethod(&window, "onThreadTreeLoaded",
+ Q_ARG(QVector<MessageNode>, nodes),
+ Q_ARG(quint64, generation));
+
+ QVERIFY2(view->isExpanded(back),
+ "the thread collapsed again once its replies arrived");
+ QCOMPARE(model->rowCount(back), 3);
+
+ const QModelIndex current = view->currentIndex();
+ QVERIFY2(current.isValid(), "recovery left nothing selected");
+ QVERIFY2(model->isMessageRow(current),
+ "recovery landed on the thread rather than on the reply the user "
+ "was reading");
+ QCOMPARE(model->messageAt(current).messageId,
+ QStringLiteral("m3@example.org"));
+}
+
+void TestMainWindow::theRecoveryButtonSurvivesThePaneBeingBlanked()
+{
+ // The defect that survived six wrong diagnoses and every other recovery
+ // test in this file, because all of them reach the slot through
+ // invokeMethod, which COPIES its arguments.
+ //
+ // MessageView emitted the signal with its own members, so a direct
+ // connection handed MainWindow::recoverStaleThread() references to them.
+ // That slot calls runCurrentQuery(), which blanks the pane, which calls
+ // setStaleThread() and assigns to those very members. The ids the slot was
+ // still holding went empty mid-call, the recovery target was stored as an
+ // empty string, and nothing was ever recovered: the thread came back
+ // collapsed with the pane blank.
+ //
+ // Driven through the real button so the real signal runs. A test that
+ // calls the slot directly cannot see this and will pass against it.
+ const Config config;
+ MainWindow window(config);
+
+ auto *queryEdit = window.findChild<QLineEdit *>();
+ QVERIFY(queryEdit);
+ auto *pane = window.findChild<MessageView *>();
+ QVERIFY(pane);
+ auto *button =
+ pane->findChild<QPushButton *>(QStringLiteral("staleThreadButton"));
+ QVERIFY(button);
+
+ queryEdit->setText(QStringLiteral("tag:unread"));
+ queryEdit->returnPressed();
+
+ pane->setStaleThread(QStringLiteral("T1"),
+ QStringLiteral("m3@example.org"));
+ QCOMPARE(pane->staleThreadId(), QStringLiteral("T1"));
+
+ button->click();
+
+ // The query the button ran is the thread's own, which is only true if the
+ // id survived the round trip.
+ QCOMPARE(queryEdit->text(), QStringLiteral("thread:T1"));
+
+ // And the target is still pending, waiting for the result. Empty here means
+ // the reference was clobbered and the recovery is already dead.
+ QVERIFY2(window.hasPendingRecoveryForTesting(),
+ "the recovery target was lost during the slot, so the thread will "
+ "come back collapsed with a blank pane");
}
void TestMainWindow::theSyncActionIsDisabledWhileABackgroundSyncHoldsTheLock()
diff --git a/tests/test_threadlistmodel.cpp b/tests/test_threadlistmodel.cpp
index aa71080..947a6b3 100644
--- a/tests/test_threadlistmodel.cpp
+++ b/tests/test_threadlistmodel.cpp
@@ -73,6 +73,14 @@ private slots:
void tagChangeForUnknownThreadIsIgnored();
void tagChangeRoundTripsForRevert();
void modelPassesQtTester();
+ void reconcileAddsNewThreadsInTheOrderGiven();
+ void reconcileRemovesThreadsThatNoLongerMatch();
+ void reconcileKeepsSurvivingRowsAndTheirExpansion();
+ void reconcileUpdatesTagsOnASurvivingThread();
+ void reconcileOnAnEmptyModelFillsIt();
+ void reconcileWithAnIdenticalResultChangesNothing();
+ void reconcileMovesAThreadBumpedByANewReply();
+ void reconcileKeepsAMovedRowsPersistentIndex();
};
static ThreadSummary makeThread(const QString &id, const QString &subject)
@@ -1172,5 +1180,245 @@ void TestThreadListModel::replySharingEveryThreadTagShowsNone()
.isEmpty());
}
+void TestThreadListModel::reconcileAddsNewThreadsInTheOrderGiven()
+{
+ // Item 35b. The auto-refresh hands the model a fresh result set and the
+ // model works out the difference, rather than being cleared and refilled.
+ //
+ // Position comes from the result, never from a rule of this model's own:
+ // the query is sorted by the worker, so with newest-first a new thread
+ // arrives at the front and with oldest-first at the back. A model that
+ // forced new rows to the top would contradict the sort the user chose.
+ ThreadListModel model;
+ model.appendBatch({ makeThread(QStringLiteral("t1"),
+ QStringLiteral("One")),
+ makeThread(QStringLiteral("t2"),
+ QStringLiteral("Two")) });
+
+ model.reconcile({ makeThread(QStringLiteral("t3"),
+ QStringLiteral("Newest")),
+ makeThread(QStringLiteral("t1"),
+ QStringLiteral("One")),
+ makeThread(QStringLiteral("t2"),
+ QStringLiteral("Two")) });
+
+ QCOMPARE(model.rowCount(), 3);
+ QCOMPARE(model.threadAt(0).threadId, QStringLiteral("t3"));
+ QCOMPARE(model.threadAt(1).threadId, QStringLiteral("t1"));
+ QCOMPARE(model.threadAt(2).threadId, QStringLiteral("t2"));
+}
+
+void TestThreadListModel::reconcileRemovesThreadsThatNoLongerMatch()
+{
+ // A thread read out of an Unread view stops matching, and the list has to
+ // say so. Leaving it would make the list disagree with its own query, and
+ // every view-wide action (Mark all read) acts on what the list holds.
+ ThreadListModel model;
+ model.appendBatch({ makeThread(QStringLiteral("t1"),
+ QStringLiteral("One")),
+ makeThread(QStringLiteral("t2"),
+ QStringLiteral("Two")) });
+
+ model.reconcile({ makeThread(QStringLiteral("t2"),
+ QStringLiteral("Two")) });
+
+ QCOMPARE(model.rowCount(), 1);
+ QCOMPARE(model.threadAt(0).threadId, QStringLiteral("t2"));
+}
+
+void TestThreadListModel::reconcileKeepsSurvivingRowsAndTheirExpansion()
+{
+ // The whole point of reconciling rather than clearing. A surviving thread
+ // must keep the SAME row identity, because the view's selection, its
+ // expanded state and the open message all hang off persistent indexes: a
+ // beginResetModel drops every one of them, which is what made the old
+ // refresh close the thread being read.
+ ThreadListModel model;
+ model.appendBatch({ makeThread(QStringLiteral("t1"),
+ QStringLiteral("One")),
+ makeThread(QStringLiteral("t2"),
+ QStringLiteral("Two")) });
+
+ MessageNode root = makeNode(QStringLiteral("m1"), 0);
+ MessageNode reply = makeNode(QStringLiteral("m2"), 1);
+ model.setThreadMessages(QStringLiteral("t1"), { root, reply });
+ QCOMPARE(model.rowCount(model.index(0, 0)), 1);
+
+ const QPersistentModelIndex survivor(model.index(0, 0));
+ QVERIFY(survivor.isValid());
+
+ // t2 leaves, t3 arrives, t1 stays put.
+ model.reconcile({ makeThread(QStringLiteral("t1"),
+ QStringLiteral("One")),
+ makeThread(QStringLiteral("t3"),
+ QStringLiteral("Three")) });
+
+ QVERIFY2(survivor.isValid(),
+ "reconciling invalidated a surviving row, so the selection and "
+ "the open thread would be lost exactly as a reset loses them");
+ QCOMPARE(model.threadAt(survivor.row()).threadId, QStringLiteral("t1"));
+
+ // Its loaded replies survive too, or the thread collapses under the reader.
+ QCOMPARE(model.rowCount(model.index(survivor.row(), 0)), 1);
+}
+
+void TestThreadListModel::reconcileUpdatesTagsOnASurvivingThread()
+{
+ // A thread that stays but changed state: read elsewhere, tagged by a
+ // filter, flagged on the phone. The row has to repaint, or the list shows
+ // stale state while claiming to be current.
+ ThreadListModel model;
+ model.appendBatch({ makeThread(QStringLiteral("t1"),
+ QStringLiteral("One")) });
+ QVERIFY(model.threadAt(0).tags.contains(QStringLiteral("unread")));
+
+ ThreadSummary readNow = makeThread(QStringLiteral("t1"),
+ QStringLiteral("One"));
+ readNow.tags = QStringList{ QStringLiteral("inbox") };
+
+ QSignalSpy changed(&model, &QAbstractItemModel::dataChanged);
+ model.reconcile({ readNow });
+
+ QCOMPARE(model.rowCount(), 1);
+ QVERIFY2(!model.threadAt(0).tags.contains(QStringLiteral("unread")),
+ "a surviving thread kept its stale tags");
+ QVERIFY2(!changed.isEmpty(),
+ "the row's new state was stored without repainting it");
+}
+
+void TestThreadListModel::reconcileOnAnEmptyModelFillsIt()
+{
+ // Item 35a's case, now reached through the same path as every other
+ // refresh rather than through a special one: read the view empty, cron
+ // indexes new mail, it appears.
+ ThreadListModel model;
+ QCOMPARE(model.rowCount(), 0);
+
+ model.reconcile({ makeThread(QStringLiteral("t1"),
+ QStringLiteral("New")) });
+
+ QCOMPARE(model.rowCount(), 1);
+ QCOMPARE(model.threadAt(0).threadId, QStringLiteral("t1"));
+}
+
+void TestThreadListModel::reconcileWithAnIdenticalResultChangesNothing()
+{
+ // The common case: the sync brought nothing this query cares about. It
+ // runs every ten minutes under a reader, so it must not churn rows, and
+ // must not emit a reset that would collapse an expanded thread.
+ ThreadListModel model;
+ model.appendBatch({ makeThread(QStringLiteral("t1"),
+ QStringLiteral("One")),
+ makeThread(QStringLiteral("t2"),
+ QStringLiteral("Two")) });
+
+ const QPersistentModelIndex kept(model.index(1, 0));
+ QSignalSpy reset(&model, &QAbstractItemModel::modelReset);
+ QSignalSpy inserted(&model, &QAbstractItemModel::rowsInserted);
+ QSignalSpy removed(&model, &QAbstractItemModel::rowsRemoved);
+
+ model.reconcile({ makeThread(QStringLiteral("t1"),
+ QStringLiteral("One")),
+ makeThread(QStringLiteral("t2"),
+ QStringLiteral("Two")) });
+
+ QCOMPARE(model.rowCount(), 2);
+ QVERIFY2(reset.isEmpty(), "an unchanged result reset the model");
+ QVERIFY2(inserted.isEmpty(), "an unchanged result inserted rows");
+ QVERIFY2(removed.isEmpty(), "an unchanged result removed rows");
+ QVERIFY(kept.isValid());
+ QCOMPARE(kept.row(), 1);
+}
+
+void TestThreadListModel::reconcileMovesAThreadBumpedByANewReply()
+{
+ // The case the other reconcile tests all miss, and the commonest reordering
+ // there is: an old thread gets a new reply, so under newest-first the
+ // worker returns it at the FRONT although it was already on screen. It is
+ // neither an arrival nor a departure, and a reconcile that only handles
+ // those two leaves it where it was, showing an order the query does not
+ // agree with.
+ ThreadListModel model;
+ model.appendBatch({ makeThread(QStringLiteral("t1"),
+ QStringLiteral("One")),
+ makeThread(QStringLiteral("t2"),
+ QStringLiteral("Two")),
+ makeThread(QStringLiteral("t3"),
+ QStringLiteral("Three")) });
+
+ // t3 was replied to and now sorts first.
+ model.reconcile({ makeThread(QStringLiteral("t3"),
+ QStringLiteral("Three")),
+ makeThread(QStringLiteral("t1"),
+ QStringLiteral("One")),
+ makeThread(QStringLiteral("t2"),
+ QStringLiteral("Two")) });
+
+ QCOMPARE(model.rowCount(), 3);
+ QCOMPARE(model.threadAt(0).threadId, QStringLiteral("t3"));
+ QCOMPARE(model.threadAt(1).threadId, QStringLiteral("t1"));
+ QCOMPARE(model.threadAt(2).threadId, QStringLiteral("t2"));
+}
+
+void TestThreadListModel::reconcileKeepsAMovedRowsPersistentIndex()
+{
+ // A reordering seen from the VIEW's side rather than the data's.
+ //
+ // reconcile() places rows with beginMoveRows, and the assertions on
+ // threadAt() cannot tell a correct move from a broken one: QVector::move
+ // reorders the storage whatever Qt was told, so the data lands right even
+ // if the signal is wrong and only a persistent index reports the
+ // difference. A real view's selection rides on exactly that.
+ //
+ // Every move reconcile() makes is upwards, which is a property of the walk
+ // and not of this data: the result is walked front to back, so rows ahead
+ // of the target are already final and a misplaced survivor is always
+ // pulled forward. The model asserts that invariant.
+ ThreadListModel model;
+
+ // Fatal, and attached BEFORE the move. The tester is what actually checks
+ // the beginMoveRows arguments against the rows that end up moving; the
+ // assertions below read m_threads, which QVector::move reorders correctly
+ // whatever destination Qt was told. Without this a wrong destination
+ // corrupts only what the VIEW is told, and every assertion here still
+ // passes while a real view's selection lands on the wrong row.
+ QAbstractItemModelTester tester(
+ &model, QAbstractItemModelTester::FailureReportingMode::Fatal);
+ Q_UNUSED(tester);
+
+ model.appendBatch({ makeThread(QStringLiteral("t1"),
+ QStringLiteral("One")),
+ makeThread(QStringLiteral("t2"),
+ QStringLiteral("Two")),
+ makeThread(QStringLiteral("t3"),
+ QStringLiteral("Three")) });
+
+ const QPersistentModelIndex moved(model.index(0, 0));
+ QVERIFY(moved.isValid());
+
+ // t1 ends last. Reached by t2 and t3 each being pulled forward past it,
+ // which is what makes t1's persistent index the thing under test: it is
+ // displaced twice without ever being the row that moves.
+ model.reconcile({ makeThread(QStringLiteral("t2"),
+ QStringLiteral("Two")),
+ makeThread(QStringLiteral("t3"),
+ QStringLiteral("Three")),
+ makeThread(QStringLiteral("t1"),
+ QStringLiteral("One")) });
+
+ QCOMPARE(model.rowCount(), 3);
+ QCOMPARE(model.threadAt(0).threadId, QStringLiteral("t2"));
+ QCOMPARE(model.threadAt(1).threadId, QStringLiteral("t3"));
+ QCOMPARE(model.threadAt(2).threadId, QStringLiteral("t1"));
+
+ // The persistent index followed the row rather than being invalidated,
+ // which is what keeps a selection on a thread that reordered under it.
+ // This is the assertion the destination adjustment is answerable to: with
+ // the wrong destination the DATA still lands correctly (QVector::move does
+ // not care what Qt was told) and only this reports the difference.
+ QVERIFY2(moved.isValid(), "a moved row lost its persistent index");
+ QCOMPARE(moved.row(), 2);
+}
+
QTEST_MAIN(TestThreadListModel)
#include "test_threadlistmodel.moc"