diff --git a/CHANGELOG.md b/CHANGELOG.md index 1263af8a..cce6456a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -90,6 +90,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 logs. The processor is now hashed by `project_id` — #296 - **Apply button**: A new value of `0` no longer disables Apply. An empty string still does; use "remove value" to blank a cell — #296 +- **Backcheck settings**: `backchecks_report_settings` passed the duplicate + option and target as `drop_duplicates` and `backcheck_goal`, which + `BackcheckSettings` silently dropped, so duplicates were always dropped and + the target was always 10. They are now passed as `drop_duplicates_option` + and `backcheck_target_percent`, and the target is saved under + `backcheck_target_percent` so it reloads in later sessions. Targets saved + under the old `backcheck_goal` key were never applied and are ignored — #299 - **Correction log schema**: Removing the last correction entry now leaves an empty log with the full schema, including status columns — #296 - **Constraint violations**: A value past a hard bound was reported as a soft diff --git a/src/datasure/checks/backchecks/settings_ui.py b/src/datasure/checks/backchecks/settings_ui.py index 4618c809..92901eee 100644 --- a/src/datasure/checks/backchecks/settings_ui.py +++ b/src/datasure/checks/backchecks/settings_ui.py @@ -260,27 +260,29 @@ def _render_tracking_options( Returns ------- int - Backcheck goal. + Backcheck target percent. """ with st.container(border=True): st.subheader("Tracking Options") to1, _, _ = st.columns(3) with to1: - backcheck_goal = st.number_input( + backcheck_target_percent = st.number_input( "Target number of backchecks", min_value=0, help="Total number of backchecks expected", key="backcheck_goal_backchecks", value=default_settings.backcheck_target_percent, on_change=trigger_save, - kwargs={"state_name": TAB_NAME + "_backcheck_goal"}, + kwargs={"state_name": TAB_NAME + "_backcheck_target_percent"}, ) save_check_settings( - settings_file, TAB_NAME, {"backcheck_goal": backcheck_goal} + settings_file, + TAB_NAME, + {"backcheck_target_percent": backcheck_target_percent}, ) - return backcheck_goal + return backcheck_target_percent def _render_duplicate_handling( @@ -500,7 +502,9 @@ def backchecks_report_settings( backcheck_categorical_columns, ) - backcheck_goal = _render_tracking_options(settings_file, default_settings) + backcheck_target_percent = _render_tracking_options( + settings_file, default_settings + ) ( drop_duplicates_option, @@ -516,8 +520,8 @@ def backchecks_report_settings( backcheck_date=backcheck_date, enumerator=enumerator, backchecker=backchecker, - backcheck_goal=backcheck_goal, - drop_duplicates=drop_duplicates_option, + backcheck_target_percent=backcheck_target_percent, + drop_duplicates_option=drop_duplicates_option, no_differences_list=no_diff_values, exclude_values_list=exclude_values, case_option=string_comp_options.case_option, diff --git a/tests/checks/backchecks/test_settings_ui.py b/tests/checks/backchecks/test_settings_ui.py index 78b05258..97f53262 100644 --- a/tests/checks/backchecks/test_settings_ui.py +++ b/tests/checks/backchecks/test_settings_ui.py @@ -4,6 +4,7 @@ import sys from unittest.mock import MagicMock, patch +import pandas as pd import pytest from datasure.checks.backchecks.models import BackcheckSettings, StrCompareOptions @@ -17,7 +18,9 @@ _render_survey_identifiers, _render_tracking_options, _render_value_list_display, + backchecks_report_settings, ) +from datasure.utils.settings_utils import load_check_settings from tests.checks.backchecks.conftest import make_mock_st # ============================================ @@ -227,3 +230,76 @@ def test_render_exclude_values_settings_fragment(bc): """_render_exclude_values_settings runs without error and returns a list.""" result = bc._render_exclude_values_settings("settings.json") assert isinstance(result, list) + + +# ============================================ +# backchecks_report_settings: user choices reach the returned settings +# ============================================ + + +@pytest.fixture +def report_settings_with_choices(patched_bc): + """Run backchecks_report_settings with non-default UI selections.""" + module = "datasure.checks.backchecks.settings_ui" + with ( + patch( + f"{module}.load_default_backchecks_settings", + return_value=BackcheckSettings(survey_key=None), + ), + patch(f"{module}._render_survey_identifiers", return_value=("key", "sid")), + patch(f"{module}._render_date_columns", return_value=(None, None)), + patch(f"{module}._render_staff_identifiers", return_value=(None, None)), + patch(f"{module}._render_tracking_options", return_value=35), + patch( + f"{module}._render_additional_options", + return_value=("last", [], [], StrCompareOptions()), + ), + ): + yield backchecks_report_settings( + "project", + "settings.json", + pd.DataFrame(), + pd.DataFrame(), + BackcheckSettings(survey_key=None), + [], + [], + [], + [], + ) + + +def test_report_settings_keeps_selected_duplicate_option( + report_settings_with_choices, +): + """A non-default duplicate-handling choice is not replaced by the default.""" + assert report_settings_with_choices.drop_duplicates_option == "last" + + +def test_report_settings_keeps_selected_target_percent( + report_settings_with_choices, +): + """A non-default backcheck target is not replaced by the default.""" + assert report_settings_with_choices.backcheck_target_percent == 35 + + +def test_render_tracking_options_persists_changed_target(tmp_path): + """A changed target passes the real save guard and reloads from disk.""" + settings_file = str(tmp_path / "settings.json") + session_state: dict = {} + mock_st = make_mock_st() + mock_st.session_state = session_state + + def change_target(*_args, on_change, kwargs, **_widget_kwargs): + on_change(**kwargs) + return 35 + + mock_st.number_input.side_effect = change_target + with ( + patch("datasure.checks.backchecks.settings_ui.st", mock_st), + patch("datasure.utils.settings_utils.st", mock_st), + ): + _render_tracking_options(settings_file, BackcheckSettings(survey_key=None)) + + assert load_check_settings(settings_file, "backchecks") == { + "backcheck_target_percent": 35 + }