fix(unified-inbox): don't discard every account's envelopes if one account fails#13204
fix(unified-inbox): don't discard every account's envelopes if one account fails#13204ktogias wants to merge 4 commits into
Conversation
…count fails fetchEnvelopes() and fetchNextEnvelopes() combine every constituent account's envelope fetch into the unified/priority inbox via Promise.all(), which rejects as soon as any single promise rejects. A single account that's temporarily unreachable (throttled provider, network hiccup, etc.) was therefore enough to make the whole unified mailbox render nothing at all, discarding every other account's already-successful envelopes too. Catches each individual account's fetch before it reaches Promise.all(), so a failing account contributes an empty list instead of failing the whole batch. Same reasoning applied to the pagination path in fetchNextEnvelopes(). Fixes nextcloud#9072 Signed-off-by: Konstantinos Togias <info@ktogias.gr>
|
You're right, and I'm sorry. I missed the guideline about the disclosure. |
… one account fails fix(unified-inbox): don't discard every account's envelopes if one account fails Fixes nextcloud#9072 Assisted-by: Claude:claude-sonnet-5 Signed-off-by: Konstantinos Togias <info@ktogias.gr>
…ilure' into fix/unified-inbox-one-account-failure
|
Hello there, We hope that the review process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR review process. Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6 Thank you for contributing to Nextcloud and we hope to hear from you soon! (If you believe you should not receive this message, you can add yourself to the blocklist.) |
|
@coderabbitai review |
✅ Action performedReview finished.
|
📝 WalkthroughWalkthroughChangesUnified mailbox resilience
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/store/mainStore/actions.js (1)
837-849: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCover the rejected pagination request path.
The existing pagination test returns
[]for the second mailbox, so it does not exercise this new.catch(). Make one constituent reject and assert the successful page is still added.src/tests/unit/store/actions.spec.js (1)
237-264: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert that the failing account was fetched.
The result assertion also passes if account 14 is skipped. Assert that
MessageService.fetchEnvelopeswas invoked for both account IDs so the rejection branch is definitely exercised.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: e2fbe548-50fe-40b4-bfe3-aefb7159cbb3
📒 Files selected for processing (2)
src/store/mainStore/actions.jssrc/tests/unit/store/actions.spec.js
Summary
fetchEnvelopes()andfetchNextEnvelopes()combine every constituent account's envelope fetch into the unified/priority inbox viaPromise.all(), which rejects as soon as any single promise rejects. A single account that's temporarily unreachable (throttled provider, network hiccup, provisioned account with an unavailable server, etc.) was therefore enough to make the whole unified mailbox render nothing at all, discarding every other account's already-successful envelopes too.This matches the failure shape described in #9072 ("Other section of unified inbox doesn't load for new accounts") — one account's IMAP server being unavailable breaking the combined view for every account.
Fix
Catches each individual account's fetch before it reaches
Promise.all(), so a failing account contributes an empty list instead of failing the whole batch. Same reasoning applied to the pagination path infetchNextEnvelopes().Test plan
npx vitest --run src/tests/unit/store/actions.spec.js— 24/24 passing.npx eslint src/store/mainStore/actions.js— 0 errors (pre-existing unrelated jsdoc warnings only).Fixes #9072
Summary by CodeRabbit