[PM-11462] [Defect] Getting MP re-prompt on edit #10836
Conversation
alec-livefront
requested review from
cd-bitwarden,
shane-melton and
gbubemismith
Codecov ReportAttention: Patch coverage is
✅ All tests successful. No failed tests found.
Additional details and impacted files@@ Coverage Diff @@
## main #10836 +/- ##
==========================================
- Coverage 35.05% 35.04% -0.01%
==========================================
Files 2711 2711
Lines 84576 84595 +19
Branches 16069 16075 +6
==========================================
Hits 29649 29649
- Misses 53956 53975 +19
Partials 971 971 ☔ View full report in Codecov by Sentry. |
|
New Issues
|
alec-livefront
changed the title
alec-livefront
changed the title
| const extensionRefreshEnabled = await firstValueFrom(this.extensionRefreshEnabled$); | ||
| // if cipher exists (cipher is null when new) and MP re-prompt | ||
| // is on for this cipher, then show password re=prompt | ||
| if (!extensionRefreshEnabled) { |
There was a problem hiding this comment.
🎨 Instead of having to check this feature flag multiple times and track the reason for the negation, could we instead pass in an argument to skip the password re-prompt (e.g. skipReprompt: boolean = false). Then, the viewCipherById callback can pass in true while the queryParams subscription above can remain unchanged (default to always re-prompting). What do you think?
| // didn't pass password prompt, so don't open add / edit modal | ||
| this.go({ cipherId: null, itemId: null }); | ||
| return; | ||
| if (!this.extensionRefreshEnabled) { |
There was a problem hiding this comment.
🎨 Same here, if we add a skipReprompt argument that defaults to false we should be able to leave everything else alone except the viewCipherById() edit action where we would pass in true (since we know they already passed the re-prompt)
| /** | ||
| * This is used to prevent the password re-prompt from being triggered if the cipher was edited from the view dialog. | ||
| */ | ||
| protected cipherEditedFromViewDialog: boolean; |
There was a problem hiding this comment.
❓ Why do we need this in the org vault and not the individual vault?
There was a problem hiding this comment.
I was able to eliminate this in the rewrite.
|
@shane-melton I've reworked this based on your suggestion of adding a I noticed an issue that was present in the original work as well: if you were to close the |
|
Will be handled by the changes in PM-12389 |
🎟️ Tracking
https://bitwarden.atlassian.net/browse/PM-11462?atlOrigin=eyJpIjoiYmJiMDhjNzQyNjk0NGEyY2ExYzNlYTMwMjI3YjFjNmMiLCJwIjoiaiJ9
📔 Objective
action="edit"in URL when page is loaded)⏰ Reminders before review
🦮 Reviewer guidelines
:+1:) or similar for great changes:memo:) or ℹ️ (:information_source:) for notes or general info:question:) for questions:thinking:) or 💭 (:thought_balloon:) for more open inquiry that's not quite a confirmed issue and could potentially benefit from discussion:art:) for suggestions / improvements:x:) or:warning:) for more significant problems or concerns needing attention:seedling:) or ♻️ (:recycle:) for future improvements or indications of technical debt:pick:) for minor or nitpick changes