From 1042df87091f2294887aae01d801804ea90cd057 Mon Sep 17 00:00:00 2001 From: Jan Sander <63044278+JanhSander@users.noreply.github.com> Date: Thu, 4 Apr 2024 16:03:00 +0200 Subject: [PATCH 1/5] New warning dialog --- src/datadoc/app.py | 2 ++ src/datadoc/frontend/callbacks/dataset.py | 22 +++++++++++++++---- .../frontend/callbacks/register_callbacks.py | 1 + src/datadoc/frontend/components/alerts.py | 13 ++++++++--- src/datadoc/frontend/components/builders.py | 7 +++++- 5 files changed, 37 insertions(+), 8 deletions(-) diff --git a/src/datadoc/app.py b/src/datadoc/app.py index 79440cbd..80cc259f 100644 --- a/src/datadoc/app.py +++ b/src/datadoc/app.py @@ -22,6 +22,7 @@ from datadoc.enums import SupportedLanguages from datadoc.frontend.callbacks.register_callbacks import register_callbacks from datadoc.frontend.components.alerts import dataset_validation_error +from datadoc.frontend.components.alerts import naming_convention_warning from datadoc.frontend.components.alerts import opened_dataset_error from datadoc.frontend.components.alerts import opened_dataset_success from datadoc.frontend.components.alerts import saved_metadata_success @@ -65,6 +66,7 @@ def build_app(app: type[Dash]) -> Dash: opened_dataset_error, saved_metadata_success, opened_dataset_success, + naming_convention_warning, dbc.Tabs( id="tabs", class_name="ssb-tabs", diff --git a/src/datadoc/frontend/callbacks/dataset.py b/src/datadoc/frontend/callbacks/dataset.py index 2f81183a..e3a5ff6e 100644 --- a/src/datadoc/frontend/callbacks/dataset.py +++ b/src/datadoc/frontend/callbacks/dataset.py @@ -9,6 +9,7 @@ from pydantic import ValidationError from datadoc import state +from datadoc.backend.dapla_dataset_path_info import DaplaDatasetPathInfo from datadoc.backend.datadoc_metadata import DataDocMetadata from datadoc.enums import ( SupportedLanguages, # noqa: TCH001 import is needed for docs build @@ -53,11 +54,10 @@ def open_file(file_path: str | None = None) -> DataDocMetadata: def open_dataset_handling( n_clicks: int, file_path: str, -) -> tuple[bool, bool, str, str]: +) -> tuple[bool, bool, bool, str, str]: """Handle errors and other logic around opening a dataset file.""" if file_path: file_path = file_path.strip() - try: state.metadata = open_file(file_path) except FileNotFoundError: @@ -65,6 +65,7 @@ def open_dataset_handling( return ( False, True, + False, f"Filen '{file_path}' finnes ikke.", state.current_metadata_language.value, ) @@ -72,13 +73,26 @@ def open_dataset_handling( return ( False, True, + False, "\n".join(traceback.format_exception_only(type(e), e)), state.current_metadata_language.value, ) + + # opened-dataset-success if n_clicks and n_clicks > 0: - return True, False, "", state.current_metadata_language.value + dapla_dataset_path_info = DaplaDatasetPathInfo(file_path) + if not dapla_dataset_path_info.path_complies_with_naming_standard(): + return ( + False, + False, + True, + "", + state.current_metadata_language.value, + ) + return True, False, False, "", state.current_metadata_language.value - return False, False, "", state.current_metadata_language.value + # no message + return False, False, False, "", state.current_metadata_language.value def process_keyword(value: str) -> list[str]: diff --git a/src/datadoc/frontend/callbacks/register_callbacks.py b/src/datadoc/frontend/callbacks/register_callbacks.py index 377190c9..0b3f0e13 100644 --- a/src/datadoc/frontend/callbacks/register_callbacks.py +++ b/src/datadoc/frontend/callbacks/register_callbacks.py @@ -110,6 +110,7 @@ def callback_accept_dataset_metadata_input( @app.callback( Output("opened-dataset-success", "is_open"), Output("opened-dataset-error", "is_open"), + Output("opened-dataset_warning", "is_open"), Output("opened-dataset-error-explanation", "children"), Output("language-dropdown", "value"), # Used to force reload of metadata Input("open-button", "n_clicks"), diff --git a/src/datadoc/frontend/components/alerts.py b/src/datadoc/frontend/components/alerts.py index d2062544..09e82341 100644 --- a/src/datadoc/frontend/components/alerts.py +++ b/src/datadoc/frontend/components/alerts.py @@ -6,21 +6,21 @@ from datadoc.frontend.components.builders import build_ssb_alert dataset_validation_error = build_ssb_alert( - AlertTypes.WARNING, + AlertTypes.ERROR, "dataset-validation-error", "Validering feilet", "dataset-validation-explanation", ) variables_validation_error = build_ssb_alert( - AlertTypes.WARNING, + AlertTypes.ERROR, "variables-validation-error", "Validering feilet", "variables-validation-explanation", ) opened_dataset_error = build_ssb_alert( - AlertTypes.WARNING, + AlertTypes.ERROR, "opened-dataset-error", "Kunne ikke åpne datasettet", "opened-dataset-error-explanation", @@ -39,3 +39,10 @@ "Åpnet datasett", "opened-dataset-success-explanation", ) + +naming_convention_warning = build_ssb_alert( + AlertTypes.WARNING, + "opened-dataset_warning", + "Filen følger ikke navnestandard", + "opened-dataset-warning-explanation", +) diff --git a/src/datadoc/frontend/components/builders.py b/src/datadoc/frontend/components/builders.py index 457e48c6..eb4ff4a2 100644 --- a/src/datadoc/frontend/components/builders.py +++ b/src/datadoc/frontend/components/builders.py @@ -26,6 +26,7 @@ class AlertTypes(Enum): SUCCESS = auto() WARNING = auto() + ERROR = auto() @dataclass @@ -42,9 +43,13 @@ def get_type(alert_type: AlertTypes) -> AlertType: ALERT_TYPES = { + AlertTypes.ERROR: AlertType( + alert_class_name="ssb-dialog error", + color="danger", + ), AlertTypes.WARNING: AlertType( alert_class_name="ssb-dialog warning", - color="danger", + color="warning", ), AlertTypes.SUCCESS: AlertType( alert_class_name="ssb-dialog", From 94782d9d4436d8835546f7ca95b7f68b60dad449 Mon Sep 17 00:00:00 2001 From: Jan Sander <63044278+JanhSander@users.noreply.github.com> Date: Thu, 4 Apr 2024 16:07:47 +0200 Subject: [PATCH 2/5] Correct flag --- src/datadoc/frontend/callbacks/dataset.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/datadoc/frontend/callbacks/dataset.py b/src/datadoc/frontend/callbacks/dataset.py index e3a5ff6e..054e70b2 100644 --- a/src/datadoc/frontend/callbacks/dataset.py +++ b/src/datadoc/frontend/callbacks/dataset.py @@ -83,7 +83,7 @@ def open_dataset_handling( dapla_dataset_path_info = DaplaDatasetPathInfo(file_path) if not dapla_dataset_path_info.path_complies_with_naming_standard(): return ( - False, + True, False, True, "", From eeb802b2f412d7bdd423255489222fe721e373f5 Mon Sep 17 00:00:00 2001 From: Jan Sander <63044278+JanhSander@users.noreply.github.com> Date: Thu, 4 Apr 2024 16:09:51 +0200 Subject: [PATCH 3/5] Formatting --- src/datadoc/frontend/callbacks/dataset.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/datadoc/frontend/callbacks/dataset.py b/src/datadoc/frontend/callbacks/dataset.py index 054e70b2..d00ece42 100644 --- a/src/datadoc/frontend/callbacks/dataset.py +++ b/src/datadoc/frontend/callbacks/dataset.py @@ -77,8 +77,6 @@ def open_dataset_handling( "\n".join(traceback.format_exception_only(type(e), e)), state.current_metadata_language.value, ) - - # opened-dataset-success if n_clicks and n_clicks > 0: dapla_dataset_path_info = DaplaDatasetPathInfo(file_path) if not dapla_dataset_path_info.path_complies_with_naming_standard(): @@ -90,7 +88,6 @@ def open_dataset_handling( state.current_metadata_language.value, ) return True, False, False, "", state.current_metadata_language.value - # no message return False, False, False, "", state.current_metadata_language.value From d98f0e8ce70f5688d022eb266e27678f23a35159 Mon Sep 17 00:00:00 2001 From: Jan Sander <63044278+JanhSander@users.noreply.github.com> Date: Fri, 5 Apr 2024 11:22:42 +0200 Subject: [PATCH 4/5] Added link and unittest --- src/datadoc/config.py | 5 +++ src/datadoc/frontend/callbacks/dataset.py | 8 +--- src/datadoc/frontend/components/alerts.py | 4 +- src/datadoc/frontend/components/builders.py | 2 + .../callbacks/test_dataset_callbacks.py | 40 ++++++++++++++++--- 5 files changed, 45 insertions(+), 14 deletions(-) diff --git a/src/datadoc/config.py b/src/datadoc/config.py index d30ccec2..824c87ba 100644 --- a/src/datadoc/config.py +++ b/src/datadoc/config.py @@ -133,3 +133,8 @@ def get_unit_code() -> int | None: def get_organisational_unit_code() -> int | None: """The code for the organisational units code list in Klass.""" return int(_get_config_item("DATADOC_ORGANISATIONAL_UNIT_CODE") or 83) + + +def get_dapla_manual_naming_standard_url() -> str | None: + """Get the URL to naming standard in the DAPLA manual.""" + return _get_config_item("DAPLA_MANUAL_NAMING_STANDARD_URL") diff --git a/src/datadoc/frontend/callbacks/dataset.py b/src/datadoc/frontend/callbacks/dataset.py index d00ece42..d38da967 100644 --- a/src/datadoc/frontend/callbacks/dataset.py +++ b/src/datadoc/frontend/callbacks/dataset.py @@ -80,13 +80,7 @@ def open_dataset_handling( if n_clicks and n_clicks > 0: dapla_dataset_path_info = DaplaDatasetPathInfo(file_path) if not dapla_dataset_path_info.path_complies_with_naming_standard(): - return ( - True, - False, - True, - "", - state.current_metadata_language.value, - ) + return (True, False, True, "", state.current_metadata_language.value) return True, False, False, "", state.current_metadata_language.value # no message return False, False, False, "", state.current_metadata_language.value diff --git a/src/datadoc/frontend/components/alerts.py b/src/datadoc/frontend/components/alerts.py index 09e82341..cbd2de2a 100644 --- a/src/datadoc/frontend/components/alerts.py +++ b/src/datadoc/frontend/components/alerts.py @@ -2,6 +2,7 @@ from __future__ import annotations +from datadoc.config import get_dapla_manual_naming_standard_url from datadoc.frontend.components.builders import AlertTypes from datadoc.frontend.components.builders import build_ssb_alert @@ -43,6 +44,7 @@ naming_convention_warning = build_ssb_alert( AlertTypes.WARNING, "opened-dataset_warning", - "Filen følger ikke navnestandard", + "Filen følger ikke navnestandard. Vennlist se", "opened-dataset-warning-explanation", + link=get_dapla_manual_naming_standard_url(), ) diff --git a/src/datadoc/frontend/components/builders.py b/src/datadoc/frontend/components/builders.py index eb4ff4a2..2e1f96b0 100644 --- a/src/datadoc/frontend/components/builders.py +++ b/src/datadoc/frontend/components/builders.py @@ -78,6 +78,7 @@ def build_ssb_alert( # noqa: PLR0913 not immediately obvious how to improve thi message: str | None = None, *, start_open: bool = False, + link: str | None = None, ) -> dbc.Alert: """Make a Dash Alert according to SSBs Design System.""" alert = AlertType.get_type(alert_type) @@ -96,6 +97,7 @@ def build_ssb_alert( # noqa: PLR0913 not immediately obvious how to improve thi id=content_identifier, children=message, ), + html.A(link, href=link, target="_blank"), ], style={"width": "70%"}, ) diff --git a/tests/frontend/callbacks/test_dataset_callbacks.py b/tests/frontend/callbacks/test_dataset_callbacks.py index 5fa81274..02f12d49 100644 --- a/tests/frontend/callbacks/test_dataset_callbacks.py +++ b/tests/frontend/callbacks/test_dataset_callbacks.py @@ -45,6 +45,11 @@ def file_path(): return "valid/path/to/file.json" +@pytest.fixture() +def file_path_without_dates(): + return "valid/path/to/person_data_v1.parquet" + + @pytest.mark.parametrize( ("metadata_identifier", "provided_value", "expected_model_value"), [ @@ -278,13 +283,13 @@ def test_open_dataset_handling_normal( ): state.current_metadata_language = SupportedLanguages.ENGLISH - opened, show_error, error_msg, language = open_dataset_handling( + opened, show_error, naming_standard, error_msg, language = open_dataset_handling( n_clicks_1, file_path, ) - assert opened assert not show_error + assert naming_standard assert error_msg == "" assert language == "en" @@ -298,12 +303,13 @@ def test_open_dataset_handling_file_not_found( state.current_metadata_language = SupportedLanguages.ENGLISH open_file_mock.side_effect = FileNotFoundError() - opened, show_error, error_msg, language = open_dataset_handling( + opened, show_error, naming_standard, error_msg, language = open_dataset_handling( n_clicks_1, file_path, ) assert not opened assert show_error + assert not naming_standard assert error_msg.startswith(f"Filen '{file_path}' finnes ikke.") assert language == "en" @@ -317,12 +323,13 @@ def test_open_dataset_handling_general_exception( state.current_metadata_language = SupportedLanguages.ENGLISH open_file_mock.side_effect = ValueError() - opened, show_error, error_msg, language = open_dataset_handling( + opened, show_error, naming_standard, error_msg, language = open_dataset_handling( n_clicks_1, file_path, ) assert not opened assert show_error + assert not naming_standard assert error_msg.startswith("ValueError") assert language == "en" @@ -333,10 +340,31 @@ def test_open_dataset_handling_no_click( file_path: str, ): state.current_metadata_language = SupportedLanguages.ENGLISH - opened, show_error, error_msg, language = open_dataset_handling(0, file_path) - + opened, show_error, naming_standard, error_msg, language = open_dataset_handling( + 0, + file_path, + ) assert not opened assert not show_error + assert not naming_standard + assert error_msg == "" + assert language == "en" + + +@patch(f"{DATASET_CALLBACKS_MODULE}.open_file") +def test_open_dataset_handling_naming_standard( + open_file_mock: Mock, # noqa: ARG001 + n_clicks_1: int, + file_path_without_dates: str, +): + state.current_metadata_language = SupportedLanguages.ENGLISH + opened, show_error, naming_standard, error_msg, language = open_dataset_handling( + n_clicks_1, + file_path_without_dates, + ) + assert opened is True + assert not show_error + assert naming_standard assert error_msg == "" assert language == "en" From 825cb44536e8f54e17b7e3a4fb570d525406a69c Mon Sep 17 00:00:00 2001 From: Jan Sander <63044278+JanhSander@users.noreply.github.com> Date: Fri, 5 Apr 2024 11:35:16 +0200 Subject: [PATCH 5/5] Bugfix --- src/datadoc/frontend/callbacks/register_callbacks.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/datadoc/frontend/callbacks/register_callbacks.py b/src/datadoc/frontend/callbacks/register_callbacks.py index 0b3f0e13..3283ed9c 100644 --- a/src/datadoc/frontend/callbacks/register_callbacks.py +++ b/src/datadoc/frontend/callbacks/register_callbacks.py @@ -119,7 +119,7 @@ def callback_accept_dataset_metadata_input( def callback_open_dataset( n_clicks: int, dataset_path: str, - ) -> tuple[bool, bool, str, str]: + ) -> tuple[bool, bool, bool, str, str]: """Open a dataset. Shows an alert on success or failure.