Skip to content

[PM-31181] fix: Toast displays after view/edit cipher actions#2586

Merged
fedemkr merged 3 commits into
mainfrom
PM-31181/display-toast-after-action-view-edit-cipher
May 5, 2026
Merged

[PM-31181] fix: Toast displays after view/edit cipher actions#2586
fedemkr merged 3 commits into
mainfrom
PM-31181/display-toast-after-action-view-edit-cipher

Conversation

@fedemkr
Copy link
Copy Markdown
Member

@fedemkr fedemkr commented Apr 27, 2026

🎟️ Tracking

PM-31181

📔 Objective

When a user performs an action (archive, soft delete, permanent delete, restore, unarchive) on the view or edit cipher screen, no toast was displayed after the screen dismissed.

Root cause: All DismissAction completion blocks in ViewItemProcessor and AddEditItemProcessor captured self weakly via [weak self]. SwiftUI tears down the view hierarchy (and deallocates the processor) before UIKit fires the dismiss-animation completion, so self is nil by the time the block runs — meaning CipherItemOperationDelegate callbacks are never called and no toast appears. The triple-dot menu was unaffected because it calls the toast handler directly without a dismiss completion.

Fix: Capture delegate as a strong local let immediately before navigating to dismiss, so the DismissAction closure no longer needs self at all. No retain cycle is introduced — the processor does not own the DismissAction.

Additionally, the previously unused performOperationAndDismiss helper was corrected (changed onDismiss from (ViewItemProcessor) -> Void to () -> Void) and is now used by permanentDeleteItem, softDeleteItem, and restoreItem to remove duplicated loading-overlay boilerplate.

@fedemkr fedemkr added the ai-review-vnext Request a Claude code review using the vNext workflow label Apr 27, 2026
@github-actions github-actions Bot added app:password-manager Bitwarden Password Manager app context t:bug Change Type - Bug labels Apr 27, 2026
@fedemkr fedemkr changed the title [PM-31181] fix: capture delegate strongly before dismiss to ensure toast displays after view/edit cipher actions [PM-31181] fix: Toast displays after view/edit cipher actions Apr 27, 2026
@fedemkr fedemkr marked this pull request as ready for review April 27, 2026 16:53
@fedemkr fedemkr requested review from a team and matt-livefront as code owners April 27, 2026 16:53
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 27, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 87.22%. Comparing base (9d93d87) to head (2f65ded).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2586      +/-   ##
==========================================
+ Coverage   87.20%   87.22%   +0.01%     
==========================================
  Files        1895     1895              
  Lines      167767   167743      -24     
==========================================
+ Hits       146304   146306       +2     
+ Misses      21463    21437      -26     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Comment thread BitwardenShared/UI/Vault/VaultItem/AddEditItem/AddEditItemProcessor.swift Outdated
@fedemkr fedemkr requested a review from matt-livefront May 1, 2026 18:05
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 1, 2026

🤖 Bitwarden Claude Code Review

Overall Assessment: APPROVE

Reviewed the toast-after-cipher-action fix, which addresses a SwiftUI/UIKit teardown race where DismissAction completion blocks ran after the processor was deallocated, causing CipherItemOperationDelegate callbacks to be skipped. The fix replaces [weak self] recursive captures with capture-list copies of the weak delegate property so the dismissal completion no longer depends on self being alive. The refactor of performOperationAndDismiss to take () -> Void and consolidate permanentDeleteItem / softDeleteItem / restoreItem removes meaningful boilerplate, and existing test coverage (plus the new test_itemDeleted and test_itemSoftDeleted) exercises the dismiss-action delegate path on both processors.

Code Review Details

No findings. The capture-list pattern is correct (the strongly-captured optional weak reference is released when the closure is released, which is short-lived and owned by DismissAction / the action helper, so no retain cycle is introduced), the refactor preserves behavior for all three call sites, and the previously raised inline suggestion on archiveItem was incorporated in commit 2f65ded93.

@fedemkr fedemkr merged commit f4e1e07 into main May 5, 2026
17 checks passed
@fedemkr fedemkr deleted the PM-31181/display-toast-after-action-view-edit-cipher branch May 5, 2026 21:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ai-review-vnext Request a Claude code review using the vNext workflow app:password-manager Bitwarden Password Manager app context t:bug Change Type - Bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants