Skip to content

Fix false unsaved changes after saving emergency access settings#425

Merged
mindmonk merged 3 commits intodevelopfrom
feature/fix-admin-emergency-access-unsaved-changes-indicator-
Apr 16, 2026
Merged

Fix false unsaved changes after saving emergency access settings#425
mindmonk merged 3 commits intodevelopfrom
feature/fix-admin-emergency-access-unsaved-changes-indicator-

Conversation

@mindmonk
Copy link
Copy Markdown
Contributor

Summary

Fix false "unsaved changes" indicator in admin emergency access settings when "let vault owners choose different key holders" is disabled

Problem

When saving with allowChoosingEmergencyCouncil = false, defaultMinMembers is persisted as selectedUsers.length instead of minMembers. The hasUnsavedChanges check compared against minMembers (the raw input value), which differed from the saved value — causing the warning to persist immediately after a successful save.

Solution

Use the same conditional expression in hasUnsavedChanges as in the save function: allowChoosing ? minMembers : selectedUsers.length

@overheadhunter
Copy link
Copy Markdown
Member

defaultMinMembers is persisted as selectedUsers.length instead of minMembers.

That's correct, because there is no "min members" field visible, when allowChoosingEmergencyCouncil = false

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 18, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 87856a85-79df-4980-b965-29915413b24a

📥 Commits

Reviewing files that changed from the base of the PR and between f032898 and 16ed8a6.

📒 Files selected for processing (1)
  • frontend/src/components/AdminSettingsEmergencyAccess.vue
🚧 Files skipped from review as they are similar to previous changes (1)
  • frontend/src/components/AdminSettingsEmergencyAccess.vue

Walkthrough

The change modifies hasUnsavedChanges in AdminSettingsEmergencyAccess.vue so defaultMinMembers is only treated as different from minMembers when allowChoosing is true; when allowChoosing is false the defaultMinMembers comparison is ignored. Previously defaultMinMembers was always compared against minMembers.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

Suggested reviewers

  • overheadhunter
  • SailReal
🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: fixing a false unsaved changes indicator in emergency access settings after saving.
Description check ✅ Passed The description is directly related to the changeset, explaining the problem and solution for the unsaved changes indicator issue.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feature/fix-admin-emergency-access-unsaved-changes-indicator-

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
frontend/src/components/AdminSettingsEmergencyAccess.vue (1)

252-259: Reduce future drift by centralizing effective defaultMinMembers.

This exact conditional is now duplicated across unsaved-check and save paths. Extracting it to one computed value would prevent this regression from reappearing.

♻️ Suggested refactor
+const effectiveDefaultMinMembers = computed(() =>
+  allowChoosing.value ? minMembers.value : selectedUsers.value.length
+);
+
 const hasUnsavedChanges = computed(() => {
   return (
     initialEmergencyAccessSettings.value.enableEmergencyAccess !== enableEmergencyAccess.value ||
     initialEmergencyAccessSettings.value.defaultRequiredEmergencyKeyShares !== requiredShares.value ||
-    initialEmergencyAccessSettings.value.defaultMinMembers !== (allowChoosing.value ? minMembers.value : selectedUsers.value.length) ||
+    initialEmergencyAccessSettings.value.defaultMinMembers !== effectiveDefaultMinMembers.value ||
     initialEmergencyAccessSettings.value.allowChoosingEmergencyCouncil !== allowChoosing.value ||
     !sameCouncilMemberIds.value
   );
 });
@@
     await backend.settings.update({
       enableEmergencyAccess: enableEmergencyAccess.value,
       defaultRequiredEmergencyKeyShares: requiredShares.value,
-      defaultMinMembers: allowChoosing.value ? minMembers.value : selectedUsers.value.length,
+      defaultMinMembers: effectiveDefaultMinMembers.value,
       allowChoosingEmergencyCouncil: allowChoosing.value,
       emergencyCouncilMemberIds: selectedUsers.value.map(u => u.id),
     });
     initialEmergencyAccessSettings.value = {
       enableEmergencyAccess: enableEmergencyAccess.value,
       defaultRequiredEmergencyKeyShares: requiredShares.value,
-      defaultMinMembers: allowChoosing.value ? minMembers.value : selectedUsers.value.length,
+      defaultMinMembers: effectiveDefaultMinMembers.value,
       allowChoosingEmergencyCouncil: allowChoosing.value,
       selectedUsers: selectedUsers.value
     };
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@frontend/src/components/AdminSettingsEmergencyAccess.vue` around lines 252 -
259, The unsaved-change logic in the hasUnsavedChanges computed duplicates the
calculation for the effective default min members; extract that logic into a new
computed (e.g., effectiveDefaultMinMembers) that returns allowChoosing.value ?
minMembers.value : selectedUsers.value.length and then replace the inline
expression initialEmergencyAccessSettings.value.defaultMinMembers !==
(allowChoosing.value ? minMembers.value : selectedUsers.value.length) with
initialEmergencyAccessSettings.value.defaultMinMembers !==
effectiveDefaultMinMembers.value; update any save-path code that repeats the
same ternary to use effectiveDefaultMinMembers.value as well so the single
computed centralizes the rule.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@frontend/src/components/AdminSettingsEmergencyAccess.vue`:
- Around line 252-259: The unsaved-change logic in the hasUnsavedChanges
computed duplicates the calculation for the effective default min members;
extract that logic into a new computed (e.g., effectiveDefaultMinMembers) that
returns allowChoosing.value ? minMembers.value : selectedUsers.value.length and
then replace the inline expression
initialEmergencyAccessSettings.value.defaultMinMembers !== (allowChoosing.value
? minMembers.value : selectedUsers.value.length) with
initialEmergencyAccessSettings.value.defaultMinMembers !==
effectiveDefaultMinMembers.value; update any save-path code that repeats the
same ternary to use effectiveDefaultMinMembers.value as well so the single
computed centralizes the rule.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 69d561eb-9801-4aab-b011-8822cd414ad7

📥 Commits

Reviewing files that changed from the base of the PR and between 293b03f and 41d642d.

📒 Files selected for processing (1)
  • frontend/src/components/AdminSettingsEmergencyAccess.vue

Comment thread frontend/src/components/AdminSettingsEmergencyAccess.vue Outdated
Co-authored-by: Sebastian Stenzel <overheadhunter@users.noreply.github.com>
@mindmonk mindmonk requested a review from overheadhunter March 25, 2026 13:56
@mindmonk mindmonk merged commit abe15b9 into develop Apr 16, 2026
10 checks passed
@mindmonk mindmonk deleted the feature/fix-admin-emergency-access-unsaved-changes-indicator- branch April 16, 2026 05:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants