From 88b98c977a7c8b7d0c17fa1a255f444db48b1720 Mon Sep 17 00:00:00 2001 From: Mat Date: Fri, 29 Nov 2024 14:22:42 +0000 Subject: [PATCH] fix: fetch lastIngested for charts, and correctly show data last updated (#1116) * fix: fetch lastIngested for charts Previously the metadata last updated date was not showing for charts. * fix: do not use lastIngested in search results Previously we were inconsistently returning lastIngested as the timestamp for some (but not all) search results. If we do display a date here, it should be the data last updated, not the metadata last updated. * fix: use the correct property for data last updated The last updated date was not displaying, as it referenced a custom property that does not exist in the pydantic model. * design: tweak margin of summary card footer without this there is a bit too much space at the bottom of the card. * fixup test * fix: hide data last updated for CaDeT We were deliberately hiding this because the dbt last modified date is misleading. * copy: distinguish between data/metadata last updated * fix test --- home/service/details.py | 5 ++ home/urns.py | 3 + .../client/datahub_client.py | 2 + .../client/graphql/getChartDetails.graphql | 1 + .../client/graphql/search.graphql | 1 - .../data_platform_catalogue/client/search.py | 8 +- .../tests/client/datahub/test_search.py | 6 +- locale/en/LC_MESSAGES/django.po | 78 ++++++++++--------- scss/_details.scss | 1 + templates/details_base.html | 6 +- templates/details_metric.html | 2 +- templates/partial/contact_info.html | 2 +- tests/home/service/test_details.py | 2 + 13 files changed, 64 insertions(+), 53 deletions(-) create mode 100644 home/urns.py diff --git a/home/service/details.py b/home/service/details.py index 39d2c0f7..a2352e45 100644 --- a/home/service/details.py +++ b/home/service/details.py @@ -7,6 +7,7 @@ from django.core.validators import URLValidator from django.utils.translation import gettext as _ +from ..urns import PlatformUrns from .base import GenericService @@ -79,6 +80,7 @@ def _get_context(self): "is_access_requirements_a_url": is_access_requirements_a_url( self.database_metadata.custom_properties.access_information.dc_access_requirements ), + "PlatformUrns": PlatformUrns, } return context @@ -119,6 +121,7 @@ def _get_context(self): "is_access_requirements_a_url": is_access_requirements_a_url( self.table_metadata.custom_properties.access_information.dc_access_requirements ), + "PlatformUrns": PlatformUrns, } def _get_template(self): @@ -162,6 +165,7 @@ def _get_context(self): "is_access_requirements_a_url": is_access_requirements_a_url( self.chart_metadata.custom_properties.access_information.dc_access_requirements ), + "PlatformUrns": PlatformUrns, } @@ -188,4 +192,5 @@ def _get_context(self): "is_access_requirements_a_url": is_access_requirements_a_url( self.dashboard_metadata.custom_properties.access_information.dc_access_requirements ), + "PlatformUrns": PlatformUrns, } diff --git a/home/urns.py b/home/urns.py new file mode 100644 index 00000000..68b11b7f --- /dev/null +++ b/home/urns.py @@ -0,0 +1,3 @@ +class PlatformUrns: + CADET = "dbt" + PLATFORM_URNS = "performance-hub" diff --git a/lib/datahub-client/data_platform_catalogue/client/datahub_client.py b/lib/datahub-client/data_platform_catalogue/client/datahub_client.py index 0ebff5b6..04081070 100644 --- a/lib/datahub-client/data_platform_catalogue/client/datahub_client.py +++ b/lib/datahub-client/data_platform_catalogue/client/datahub_client.py @@ -338,6 +338,7 @@ def get_chart_details(self, urn) -> Chart: glossary_terms = parse_glossary_terms(response) created = parse_data_created(properties) modified = parse_data_last_modified(properties) + last_ingested = parse_metadata_last_ingested(response) name, display_name, qualified_name = parse_names(response, properties) parent_relations = parse_relations( @@ -361,6 +362,7 @@ def get_chart_details(self, urn) -> Chart: glossary_terms=glossary_terms, created=created, data_last_modified=modified, + metadata_last_ingested=last_ingested, platform=EntityRef(display_name=platform_name, urn=platform_name), custom_properties=custom_properties, ) diff --git a/lib/datahub-client/data_platform_catalogue/client/graphql/getChartDetails.graphql b/lib/datahub-client/data_platform_catalogue/client/graphql/getChartDetails.graphql index d6fdb47c..c030e55b 100644 --- a/lib/datahub-client/data_platform_catalogue/client/graphql/getChartDetails.graphql +++ b/lib/datahub-client/data_platform_catalogue/client/graphql/getChartDetails.graphql @@ -70,6 +70,7 @@ query getChartDetails($urn: String!) { actor } } + lastIngested } } diff --git a/lib/datahub-client/data_platform_catalogue/client/graphql/search.graphql b/lib/datahub-client/data_platform_catalogue/client/graphql/search.graphql index ab9653d0..9f95df9f 100644 --- a/lib/datahub-client/data_platform_catalogue/client/graphql/search.graphql +++ b/lib/datahub-client/data_platform_catalogue/client/graphql/search.graphql @@ -215,7 +215,6 @@ query Search( } } } - lastIngested domain { domain { urn diff --git a/lib/datahub-client/data_platform_catalogue/client/search.py b/lib/datahub-client/data_platform_catalogue/client/search.py index 5099d934..4d00515c 100644 --- a/lib/datahub-client/data_platform_catalogue/client/search.py +++ b/lib/datahub-client/data_platform_catalogue/client/search.py @@ -12,7 +12,6 @@ parse_data_owner, parse_domain, parse_glossary_terms, - parse_metadata_last_ingested, parse_names, parse_properties, parse_tags, @@ -278,7 +277,6 @@ def _parse_dataset( properties, custom_properties = parse_properties(entity) tags = parse_tags(entity) terms = parse_glossary_terms(entity) - last_ingested = parse_metadata_last_ingested(entity) name, display_name, qualified_name = parse_names(entity, properties) container = entity.get("container") if container: @@ -319,7 +317,7 @@ def _parse_dataset( metadata=metadata, tags=tags, glossary_terms=terms, - last_modified=modified or last_ingested, + last_modified=modified, ) def _parse_facets(self, facets: list[dict[str, Any]]) -> SearchFacets: @@ -422,8 +420,8 @@ def _parse_container( """ tags = parse_tags(entity) terms = parse_glossary_terms(entity) - last_ingested = parse_metadata_last_ingested(entity) properties, custom_properties = parse_properties(entity) + modified = parse_data_last_modified(properties) domain = parse_domain(entity) owner = parse_data_owner(entity) name, display_name, qualified_name = parse_names(entity, properties) @@ -449,7 +447,7 @@ def _parse_container( metadata=metadata, tags=tags, glossary_terms=terms, - last_modified=last_ingested, + last_modified=modified, ) def _parse_types_and_sub_types(self, entity: dict, entity_type: str) -> dict: diff --git a/lib/datahub-client/tests/client/datahub/test_search.py b/lib/datahub-client/tests/client/datahub/test_search.py index 93b31dbe..a7513e2e 100644 --- a/lib/datahub-client/tests/client/datahub/test_search.py +++ b/lib/datahub-client/tests/client/datahub/test_search.py @@ -1,11 +1,9 @@ -from datetime import datetime, timezone from unittest.mock import MagicMock import pytest from data_platform_catalogue.client.search import SearchClient from data_platform_catalogue.entities import ( - Audience, AccessInformation, DataSummary, EntityRef, @@ -481,7 +479,7 @@ def test_full_page(mock_graph, searcher): "row_count": "", }, tags=[], - last_modified=1705990502353, + last_modified=None, created=None, ), SearchResult( @@ -965,7 +963,7 @@ def test_get_glossary_terms(mock_graph, searcher): mock_graph.execute_graphql = MagicMock(return_value=datahub_response) response = searcher.get_glossary_terms(count=2) - print(response) + assert response == SearchResponse( total_results=2, page_results=[ diff --git a/locale/en/LC_MESSAGES/django.po b/locale/en/LC_MESSAGES/django.po index 4e8cb572..8d1a9916 100644 --- a/locale/en/LC_MESSAGES/django.po +++ b/locale/en/LC_MESSAGES/django.po @@ -5,7 +5,7 @@ msgid "" msgstr "" "Project-Id-Version: Find MoJ data\n" "Report-Msgid-Bugs-To: \n" -"POT-Creation-Date: 2024-11-04 09:47+0000\n" +"POT-Creation-Date: 2024-11-29 11:43+0000\n" "Language: en\n" "MIME-Version: 1.0\n" "Content-Type: text/plain; charset=UTF-8\n" @@ -114,11 +114,11 @@ msgstr "Subject area" msgid "filter-refresh" msgstr "selection will trigger the filter and refresh the search results" -#: home/service/details.py:72 templates/partial/search_result.html:20 +#: home/service/details.py:73 templates/partial/search_result.html:20 msgid "Database" msgstr "Database" -#: home/service/details.py:155 templates/partial/search_result.html:24 +#: home/service/details.py:158 templates/partial/search_result.html:24 msgid "Chart" msgstr "Chart" @@ -163,7 +163,7 @@ msgstr "Title" msgid "Name" msgstr "Name" -#: home/service/search.py:240 templates/details_base.html:47 +#: home/service/search.py:240 templates/details_base.html:48 #: templates/details_dashboard.html:14 templates/details_database.html:14 #: templates/details_table.html:15 msgid "Description" @@ -185,7 +185,7 @@ msgstr "Available on" msgid "Qualified name" msgstr "Qualified name" -#: home/views.py:30 templates/base/navigation.html:64 +#: home/views.py:36 templates/base/navigation.html:64 msgid "Home" msgstr "Home" @@ -269,7 +269,7 @@ msgstr "Feedback" msgid "Sign out" msgstr "Sign out" -#: templates/base/navigation.html:69 templates/details_base.html:19 +#: templates/base/navigation.html:69 templates/details_base.html:20 msgid "Search" msgstr "Search" @@ -281,29 +281,29 @@ msgstr "User guide" msgid "Contact us" msgstr "Contact us" -#: templates/details_base.html:53 +#: templates/details_base.html:54 msgid "No description available." msgstr "No description available." -#: templates/details_base.html:62 +#: templates/details_base.html:63 msgid "First created:" -msgstr "First created:" +msgstr "Data first created:" -#: templates/details_base.html:68 +#: templates/details_base.html:69 msgid "Last updated:" -msgstr "Last updated:" +msgstr "Data last updated:" -#: templates/details_base.html:79 templates/partial/search_result.html:39 +#: templates/details_base.html:92 templates/partial/search_result.html:39 msgid "Domain:" msgstr "Subject area:" -#: templates/details_base.html:80 templates/partial/contact_info.html:17 -#: templates/partial/contact_info.html:46 +#: templates/details_base.html:93 templates/partial/contact_info.html:15 +#: templates/partial/contact_info.html:42 #: templates/partial/search_result.html:40 msgid "Not provided" msgstr "Not provided." -#: templates/details_chart.html:8 templates/details_dashboard.html:45 +#: templates/details_chart.html:8 templates/details_dashboard.html:50 msgid "Access this data" msgstr "Access this data" @@ -312,11 +312,11 @@ msgstr "Access this data" msgid "\"%(display_name)s\" on %(platform_name)s" msgstr "" -#: templates/details_chart.html:11 templates/details_dashboard.html:48 +#: templates/details_chart.html:11 templates/details_dashboard.html:53 msgid "(opens in new tab)" msgstr "(opens in new tab)" -#: templates/details_dashboard.html:10 templates/details_dashboard.html:37 +#: templates/details_dashboard.html:10 templates/details_dashboard.html:42 msgid "Dashboard content" msgstr "Dashboard content" @@ -325,10 +325,14 @@ msgid "Chart name" msgstr "Chart name" #: templates/details_dashboard.html:38 +msgid "Download chart descriptions (CSV format)" +msgstr "" + +#: templates/details_dashboard.html:43 msgid "This dashboard is missing chart information." msgstr "This dashboard is missing table information." -#: templates/details_database.html:10 templates/details_database.html:43 +#: templates/details_database.html:10 templates/details_database.html:48 msgid "Database content" msgstr "Database content" @@ -337,10 +341,14 @@ msgid "Table name" msgstr "Table name" #: templates/details_database.html:44 +msgid "Download table descriptions (CSV format)" +msgstr "" + +#: templates/details_database.html:49 msgid "This database is missing table information." msgstr "This database is missing table information." -#: templates/details_table.html:11 templates/details_table.html:44 +#: templates/details_table.html:11 templates/details_table.html:50 msgid "Table schema" msgstr "Table schema" @@ -352,19 +360,23 @@ msgstr "Type" msgid "Is Nullable" msgstr "Is nullable" -#: templates/details_table.html:45 +#: templates/details_table.html:46 +msgid "Download table schema (CSV format)" +msgstr "" + +#: templates/details_table.html:51 msgid "The schema for this table is not available." msgstr "The schema for this table is not available." -#: templates/details_table.html:48 +#: templates/details_table.html:54 msgid "Lineage" msgstr "Lineage" -#: templates/details_table.html:50 +#: templates/details_table.html:56 msgid "See where this data came from and how it is used in other tables." msgstr "See where this data came from and how it is used in other tables." -#: templates/details_table.html:54 +#: templates/details_table.html:60 msgid "View lineage in DataHub (opens in new tab)" msgstr "View lineage in DataHub (opens in new tab)" @@ -431,30 +443,20 @@ msgstr "Select this link for access information (opens in new tab)" msgid "Please contact the data custodian for access information." msgstr "Contact the data custodian to request access." -#: templates/partial/contact_info.html:15 -msgid "Please contact the data owner for access information." -msgstr "Contact the data owner to request access." - -#: templates/partial/contact_info.html:24 +#: templates/partial/contact_info.html:22 msgid "Contact channels for questions" msgstr "Ask a question" -#: templates/partial/contact_info.html:42 +#: templates/partial/contact_info.html:40 msgid "Contact the data custodian with questions." msgstr "" -#: templates/partial/contact_info.html:44 -msgid "Contact the data owner with questions." -msgstr "Contact the data owner with questions." - -#: templates/partial/contact_info.html:54 +#: templates/partial/contact_info.html:50 +#: templates/partial/contact_info.html:57 msgid "Data custodian" msgstr "Data custodian (technical contact)" -#: templates/partial/contact_info.html:61 -msgid "IAO or Data Owner" -msgstr "Data owner" - +#: templates/partial/contact_info.html:64 msgid "" "Not provided - {{entity.created | date:"jS F Y"}} ({{entity.created|naturaltime}}) {% endif %} - {% if entity.custom_properties.data_summary.last_updated %} + {% if entity.data_last_modified and entity.platform.urn != PlatformUrns.CADET %}
  • {% translate "Last updated:" %} - {{entity.custom_properties.data_summary.last_updated}} + {{entity.data_last_modified | date:"jS F Y"}}
    • {% endif %} {% if entity.custom_properties.data_summary.refresh_period %} @@ -107,7 +107,7 @@

    {% endblock metadata_list %} {% if entity.metadata_last_ingested %}
    - Updated {{entity.metadata_last_ingested|timesince|format_timesince}} ago + Entry last updated {{entity.metadata_last_ingested|timesince|format_timesince}} ago
    {% endif %} diff --git a/templates/details_metric.html b/templates/details_metric.html index 037de16a..7c81f2b0 100644 --- a/templates/details_metric.html +++ b/templates/details_metric.html @@ -1,7 +1,7 @@ {% extends "details_base.html" %} {% block extra_description %} -{% if entity.platform.urn == 'performance-hub' %} +{% if entity.platform.urn == PlatformUrns.PERFORMANCE_HUB %}

    Data quality: the data is checked to be good enough to support the outcomes it is being used for. Data values should be right, but there are other factors that help ensure data meets the needs of its users.

    Please remember that this data is for INTERNAL USE ONLY and not to be shared outside the organisation.

    diff --git a/templates/partial/contact_info.html b/templates/partial/contact_info.html index 41527812..070d21d6 100644 --- a/templates/partial/contact_info.html +++ b/templates/partial/contact_info.html @@ -58,7 +58,7 @@

    {% translate "Data custodian

    {% if governance.data_owner.email %} {{ governance.data_owner.email|urlize }} - {% elif platform.urn == 'performance-hub' %} + {% elif platform.urn == PlatformUrns.PERFORMANCE_HUB %} No owner is listed as this data is undergoing a review of ownership. {% else %} {% blocktranslate %}Not provided - contact the Data Catalogue team about this data.{% endblocktranslate %} diff --git a/tests/home/service/test_details.py b/tests/home/service/test_details.py index 1334f279..148b2a53 100644 --- a/tests/home/service/test_details.py +++ b/tests/home/service/test_details.py @@ -22,6 +22,7 @@ _parse_parent, is_access_requirements_a_url, ) +from home.urns import PlatformUrns from tests.conftest import ( generate_chart_metadata, generate_dashboard_metadata, @@ -219,6 +220,7 @@ def test_get_context(self, mock_catalogue): "parent_type": "dashboard", "h1_value": "test", "is_access_requirements_a_url": False, + "PlatformUrns": PlatformUrns, } assert context == expected