diff --git a/app/api/settings.py b/app/api/settings.py index 97688f5e..555f8a82 100644 --- a/app/api/settings.py +++ b/app/api/settings.py @@ -434,12 +434,13 @@ async def rollback_setting_to_history( admin: AdminUser, ): """ - Revert a setting to the value it held at a specific point in the audit log. + Revert a setting to the value it had *before* a specific audit log change. The ``history_id`` is the ID of the :class:`~app.models.SettingsAuditLog` - entry whose ``new_value`` should be reinstated. If that entry recorded a - deletion (``new_value`` is ``None``), the setting is removed from the - database and reverts to its ENV/default value. + entry whose ``old_value`` should be reinstated, effectively undoing that + change. If ``old_value`` is ``None`` (the setting did not exist before + that change), the setting is removed from the database and reverts to its + ENV/default value. A new audit log entry is written to record the rollback. Admin only. diff --git a/app/utils/settings_service.py b/app/utils/settings_service.py index b7344118..fb813bbb 100644 --- a/app/utils/settings_service.py +++ b/app/utils/settings_service.py @@ -1201,19 +1201,20 @@ def get_setting_history(db: Session, key: str) -> List[Dict[str, Any]]: def rollback_setting(db: Session, key: str, history_id: int, changed_by: str = "system") -> bool: """ - Revert a setting to the value recorded in a specific audit log entry. + Revert a setting to the value it had *before* a specific audit log entry. - The value stored in the chosen history entry's ``new_value`` field is - re-applied as the current database value. If that value is ``None`` - (i.e. the entry recorded a deletion) the setting is removed from the - database entirely, reverting to ENV/defaults. + The value stored in the chosen history entry's ``old_value`` field is + re-applied as the current database value, effectively undoing that change. + If ``old_value`` is ``None`` (i.e. the setting did not exist before that + change) the setting is removed from the database entirely, reverting to + ENV/defaults. A new audit log entry is written to record the rollback operation. Args: db: Database session key: Setting key to roll back - history_id: ID of the SettingsAuditLog entry whose ``new_value`` + history_id: ID of the SettingsAuditLog entry whose ``old_value`` should become the restored value changed_by: Username performing the rollback (for audit log) @@ -1229,10 +1230,10 @@ def rollback_setting(db: Session, key: str, history_id: int, changed_by: str = " logger.warning(f"Rollback failed: audit log entry {history_id} not found for key '{key}'") return False - target_value = history_entry.new_value + target_value = history_entry.old_value if target_value is None: - # The history entry recorded a deletion – reinstate that by deleting the current db value + # The old value was empty – remove the current db value to revert to ENV/default return delete_setting_from_db(db, key, changed_by=changed_by) else: return save_setting_to_db(db, key, target_value, changed_by=changed_by) diff --git a/frontend/templates/audit_log.html b/frontend/templates/audit_log.html index fa16bac6..067cc0b7 100644 --- a/frontend/templates/audit_log.html +++ b/frontend/templates/audit_log.html @@ -73,7 +73,7 @@ @@ -330,6 +330,10 @@ function settingsApp() { }, async revertSetting(key) { + if (!confirm(`Remove '${key}' from the database?\n\nThe setting will revert to its environment variable or default value.`)) { + return; + } + this.revertingKey = key; this.hideAlert(); diff --git a/tests/test_settings_audit_log.py b/tests/test_settings_audit_log.py index 79021423..8f14ed8e 100644 --- a/tests/test_settings_audit_log.py +++ b/tests/test_settings_audit_log.py @@ -203,19 +203,36 @@ class TestRollbackSetting: """rollback_setting reinstates the value from a given audit log entry.""" def test_rollback_to_previous_value(self, db_session): + """Rolling back an entry restores the old_value (the value *before* that change).""" from app.utils.settings_service import get_setting_from_db, rollback_setting, save_setting_to_db - save_setting_to_db(db_session, "workdir", "/v1", changed_by="admin") # entry id 1 - save_setting_to_db(db_session, "workdir", "/v2", changed_by="admin") # entry id 2 + save_setting_to_db(db_session, "workdir", "/v1", changed_by="admin") # entry 1: old=None, new=/v1 + save_setting_to_db(db_session, "workdir", "/v2", changed_by="admin") # entry 2: old=/v1, new=/v2 - first_entry = db_session.query(SettingsAuditLog).filter_by(key="workdir").first() - # first entry has new_value="/v1" - success = rollback_setting(db_session, "workdir", first_entry.id, changed_by="rollbacker") + # Rolling back entry 2 should undo the /v1→/v2 change and restore /v1 + second_entry = ( + db_session.query(SettingsAuditLog).filter_by(key="workdir").order_by(SettingsAuditLog.id.desc()).first() + ) + success = rollback_setting(db_session, "workdir", second_entry.id, changed_by="rollbacker") assert success is True current = get_setting_from_db(db_session, "workdir") assert current == "/v1" + def test_rollback_deletes_setting_when_old_value_is_none(self, db_session): + """Rolling back the first-ever entry (old_value=None) deletes the setting from DB.""" + from app.utils.settings_service import get_setting_from_db, rollback_setting, save_setting_to_db + + save_setting_to_db(db_session, "workdir", "/v1", changed_by="admin") # old=None, new=/v1 + + first_entry = db_session.query(SettingsAuditLog).filter_by(key="workdir").first() + success = rollback_setting(db_session, "workdir", first_entry.id, changed_by="rollbacker") + + assert success is True + # Setting should be removed from DB (fallback to ENV/default) + current = get_setting_from_db(db_session, "workdir") + assert current is None + def test_rollback_creates_new_audit_entry(self, db_session): from app.utils.settings_service import rollback_setting, save_setting_to_db