Skip to content

Fix Backup & Recovery save button never enabling - #1355

Merged
Paul Lizer (paullizer) merged 2 commits into
Developmentfrom
paullizer-backup-toggle-save-issue
Aug 25, 2026
Merged

Fix Backup & Recovery save button never enabling#1355
Paul Lizer (paullizer) merged 2 commits into
Developmentfrom
paullizer-backup-toggle-save-issue

Conversation

@paullizer

Copy link
Copy Markdown
Contributor

Fixes #1353

Summary

Toggling Enable scheduled backups in Admin Settings → Backup, Migrate & Restore never enabled the Save Settings button — it stayed greyed out and labelled "Saved", so the setting could not be persisted.

The reported toggle turned out to be the visible tip of a much wider failure. The entire Backup & Recovery JavaScript module was inert: saved settings were never fetched, the backup inventory and job history never populated, and no button in any of the five tabs was wired up.

Root cause

Commit 9bbd7bda ("Stage D part 6: Backup & Recovery") split the single admin/_panes/data-management.html pane into five sibling panes — backup, migrate, restore, cosmos-editor, jobs — and moved the shared save button, status line and warning into a data-admin-group-shared="backup-recovery" region outside the panes.

admin_data_management.js was not updated. It still resolved its root container from the id the split removed, and hard-stopped when it was missing:

// bindElements()
elements.tabPane = elements.dataManagement;   // getElementById("data-management") -> null

// DOMContentLoaded
bindElements();
if (!elements.tabPane) {
    return;                                   // module dies here
}

bindEvents() therefore never ran. bindDataManagementChangeTracking() was also scoped to elements.tabPane, so no change/input listener was ever attached, dataManagementModified stayed false, and updateDataManagementSaveButtonState() kept the button disabled.

Measured: of the 214 element ids bindElements() binds, exactly onedata-management — was absent from the composed Admin Settings template.

The same removed id had a second consumer. admin_settings.js updateSaveButtonState() hides the global Save Settings button while a Backup & Recovery tab is active:

const dataManagementPane = document.getElementById('data-management');
const isDataManagementActive = Boolean(dataManagementPane?.classList.contains('active'));

That lookup was always null, so the global save button appeared on all five Backup & Recovery tabs alongside the dedicated one — two save buttons, only one of which applied to backup settings.

Approach

Re-introducing an id="data-management" wrapper is not viable: Bootstrap hides inactive panes through .tab-content > .tab-pane, a direct-child selector, so nesting the five panes one level deeper would stop that rule matching and display every pane at once.

Instead the group membership is declared in the markup and both consumers read it. Each pane carries data-admin-group-pane="backup-recovery", mirroring the existing data-admin-group-shared="backup-recovery" convention and tying the panes to the real navigation group id in admin_settings_nav.py. That also gives the functional tests a contract to enforce, so splitting or adding a Backup & Recovery tab in future cannot silently strand the module again.

Changes

File Change
templates/admin/_panes/backup.html Added data-admin-group-pane="backup-recovery"
templates/admin/_panes/migrate.html Added data-admin-group-pane="backup-recovery"
templates/admin/_panes/restore.html Added data-admin-group-pane="backup-recovery"
templates/admin/_panes/cosmos-editor.html Added data-admin-group-pane="backup-recovery"
templates/admin/_panes/jobs.html Added data-admin-group-pane="backup-recovery"
static/js/admin/admin_data_management.js Resolve every declared pane, guard on the collection, bind change tracking across all panes
static/js/admin/admin_settings.js Detect the active Backup & Recovery pane through the group attribute
config.py VERSION = "0.261.001"
functional_tests/test_admin_data_management_pane_binding.py New regression test
functional_tests/test_data_management_security_patterns.py Updated markup and save button assertions
docs/explanation/fixes/BACKUP_SETTINGS_SAVE_BUTTON_FIX.md Fix documentation

The Cosmos Editor [data-ignore-data-management-change] opt-out is preserved, so a direct database editor still does not count as a settings edit.

Testing

New functional_tests/test_admin_data_management_pane_binding.py covers:

  1. Every element id bound in bindElements() resolves to a real element in the composed template — the general assertion that catches this class of bug.
  2. Every tab in the backup-recovery navigation group has a pane declaring the group attribute.
  3. No pane outside the group claims it.
  4. The module resolves panes from the attribute, guards on the collection, and no longer references the removed id.
  5. Change tracking iterates every pane and keeps the Cosmos Editor opt-out.
  6. Settings controls that saveDataManagementSettings() sends, sampled across both the Backup and Migrate tabs, sit inside a tracked pane.
  7. The global save button defers to the group attribute.
  8. config.py VERSION is at least 0.261.001.

Run against the pre-fix state it fails 7 of 8, including the check that names the reported symptom directly:

Test failed: Settings controls that saveDataManagementSettings() sends are outside every
tracked pane, so changing them cannot enable the save button: ['data_management_enabled',
'data_management_retention_value', 'data_management_encryption_enabled']

After the fix, 13 suites pass:

PASS  test_admin_data_management_pane_binding.py
PASS  test_data_management_security_patterns.py
PASS  test_admin_settings_group_shared_regions.py
PASS  test_admin_settings_template_composition.py
PASS  test_admin_settings_modal_placement.py
PASS  test_admin_settings_field_contract.py
PASS  test_admin_settings_nav_map.py
PASS  test_admin_settings_sidebar_card_parity.py
PASS  test_admin_settings_pane_variable_scope.py
PASS  test_admin_settings_walkthrough_targets.py
PASS  test_admin_settings_dependencies.py
PASS  test_docs_app_surface_coverage.py
PASS  test_docs_site_quality.py

Before and after

Behaviour Before After
Toggling Enable scheduled backups Save button stays disabled, reads "Saved" Save button enables, reads "Save Settings"
Stored backup settings on page load Never fetched; controls show template defaults Loaded from /api/admin/data-management/settings
Backup inventory and job history Never populate Populate on load
Buttons across the five tabs Unbound and inert Wired up
Save buttons shown on a Backup & Recovery tab Two, the global one being inapplicable One, the group's own

Notes

  • Version is deliberately set to 0.261.001 and left there for this work, rather than patch-bumped per change.
  • No settings key, admin tab, action plugin or chat control was added or renamed, so docs/_data/app_surface.yml needs no regeneration.
  • support_menu_config.py still links to #data-management, which keeps working through the LEGACY_TAB_REDIRECTS map in admin_sidebar_nav.js (data-managementbackup). Left unchanged.

Pre-existing issues found but not addressed

Both are documented in the fix doc and confirmed to predate this change:

  • functional_tests/test_admin_settings_tab_preservation.py fails on Missing required HTML elements: ['class="tab-pane fade']. It reads the raw admin_settings.html rather than the composed template, so it broke when the panes moved into includes. Verified: fails identically with these changes stashed.
  • Around twenty functional tests hard-assert an exact config.py version literal, the pattern the versioning instructions call out and that test_support.versioning.assert_app_version_at_least exists to replace. They were already failing — the highest literal in the suite is 0.260.023 while config.py was 0.260.028. Verified the bump to 0.261.001 breaks none of them further, since no test asserts either version.
  • Separately, a probe found roughly 39 getElementById literals in admin_settings.js with no matching id in the composed template (for example workspaces, agents-tab, enable_group_creation_setting). Some look like stale leftovers from the same information-architecture rework; others resolve against templates outside Admin Settings. Worth its own audit.

Paul Lizer (paullizer) and others added 2 commits August 25, 2026 16:25
Splitting the Data Management tab into five panes removed the element
admin_data_management.js resolved its root from, so the module returned at
startup and bound nothing. Toggling scheduled backups left the save button
disabled, and the rest of the surface never loaded either.

Panes now declare data-admin-group-pane="backup-recovery", the module binds
across all of them, and the global save button uses the same attribute
instead of the removed id.

Fixes #1353

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@paullizer
Paul Lizer (paullizer) merged commit 234d09c into Development Aug 25, 2026
11 of 12 checks passed
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.

1 participant