From 40f4a4c2992c864284c00aff64a6734c96a2171c Mon Sep 17 00:00:00 2001 From: juuso-j <68938778+juuso-j@users.noreply.github.com> Date: Mon, 1 Jul 2024 10:32:54 +0300 Subject: [PATCH 1/3] Replace CONTRACT_TYPE wwith displayed_service_owner field --- services/api.py | 22 +++++--- .../0102_unit_new_contract_type_fields.py | 54 +++++++++++++++++++ services/models/unit.py | 37 +------------ services/search/tests/conftest.py | 5 +- services/search/tests/test_api.py | 6 +-- services/translation.py | 1 + 6 files changed, 80 insertions(+), 45 deletions(-) create mode 100644 services/migrations/0102_unit_new_contract_type_fields.py diff --git a/services/api.py b/services/api.py index 9f57361ee..5020b425f 100644 --- a/services/api.py +++ b/services/api.py @@ -40,7 +40,7 @@ UnitIdentifier, UnitServiceDetails, ) -from services.models.unit import CONTRACT_TYPES, ORGANIZER_TYPES, PROVIDER_TYPES +from services.models.unit import ORGANIZER_TYPES, PROVIDER_TYPES from services.utils import check_valid_concrete_field if settings.REST_FRAMEWORK and settings.REST_FRAMEWORK["DEFAULT_RENDERER_CLASSES"]: @@ -650,13 +650,14 @@ def get_organizer_type(self, obj): return choicefield_string(ORGANIZER_TYPES, "organizer_type", obj) def get_contract_type(self, obj): - key = choicefield_string(CONTRACT_TYPES, "contract_type", obj) + key = getattr(obj, "displayed_service_owner_type") if not key: return None - translations = {} - for lang in LANGUAGES: - with translation.override(lang): - translations[lang] = translation.gettext(key) + translations = { + "fi": getattr(obj, "displayed_service_owner_fi"), + "sv": getattr(obj, "displayed_service_owner_sv"), + "en": getattr(obj, "displayed_service_owner_en"), + } return {"id": key, "description": translations} def to_representation(self, obj): @@ -1028,6 +1029,15 @@ def validate_service_node_ids(service_node_ids): arg = filters["address"] + r"($|\s|,|[a-zA-Z]).*" queryset = queryset.filter(**{key: arg}) + if "no_private_services" in filters: + private_enterprise_value = ( + 10 # Value for PRIVATE_ENTERPRISE from ORGANIZER_TYPES + ) + queryset = queryset.exclude( + Q(displayed_service_owner_type__iexact="PRIVATE_SERVICE") + | Q(organizer_type=private_enterprise_value) + ) + maintenance_organization = self.request.query_params.get( "maintenance_organization" ) diff --git a/services/migrations/0102_unit_new_contract_type_fields.py b/services/migrations/0102_unit_new_contract_type_fields.py new file mode 100644 index 000000000..ab9e81d3e --- /dev/null +++ b/services/migrations/0102_unit_new_contract_type_fields.py @@ -0,0 +1,54 @@ +# Generated by Django 4.1.13 on 2024-07-01 06:22 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("services", "0101_exclusionword"), + ] + + operations = [ + migrations.RemoveField( + model_name="unit", + name="contract_type", + ), + migrations.AddField( + model_name="unit", + name="displayed_service_owner", + field=models.CharField(max_length=120, null=True), + ), + migrations.AddField( + model_name="unit", + name="displayed_service_owner_en", + field=models.CharField(max_length=120, null=True), + ), + migrations.AddField( + model_name="unit", + name="displayed_service_owner_fi", + field=models.CharField(max_length=120, null=True), + ), + migrations.AddField( + model_name="unit", + name="displayed_service_owner_sv", + field=models.CharField(max_length=120, null=True), + ), + migrations.AddField( + model_name="unit", + name="displayed_service_owner_type", + field=models.CharField(max_length=100, null=True), + ), + migrations.AlterField( + model_name="unit", + name="deleted_at", + field=models.DateTimeField( + blank=True, null=True, verbose_name="Poistettu-aikaleima" + ), + ), + migrations.AlterField( + model_name="unit", + name="is_active", + field=models.BooleanField(default=True, verbose_name="Aktiivinen"), + ), + ] diff --git a/services/models/unit.py b/services/models/unit.py index b3bda4f25..7100a0985 100644 --- a/services/models/unit.py +++ b/services/models/unit.py @@ -46,39 +46,6 @@ (11, "UNKNOWN"), ) -CONTRACT_TYPES = ( - (0, "private_contract_school"), - (1, "municipal_service"), - (2, "private_service"), - (3, "purchased_service"), - (4, "service_by_joint_municipal_authority"), - (5, "service_by_municipal_group_entity"), - (6, "service_by_municipally_owned_company"), - (7, "service_by_other_municipality"), - (8, "service_by_regional_cooperation_organization"), - (9, "state_service"), - (10, "supported_operations"), - (11, "voucher_service"), - (12, "state_contract_school"), -) - -CONTRACT_TYPES_TRANSLATED = ( - (0, _("private_contract_school")), - (1, _("municipal_service")), - (2, _("private_service")), - (3, _("purchased_service")), - (4, _("service_by_joint_municipal_authority")), - (5, _("service_by_municipal_group_entity")), - (6, _("service_by_municipally_owned_company")), - (7, _("service_by_other_municipality")), - (8, _("service_by_regional_cooperation_organization")), - (9, _("state_service")), - (10, _("supported_operations")), - (11, _("voucher_service")), - (12, _("state_contract_school")), -) - - _unit_related_fields = set() @@ -146,8 +113,6 @@ class Unit(SoftDeleteModel): organizer_business_id = models.CharField(max_length=10, null=True) provider_type = models.PositiveSmallIntegerField(choices=PROVIDER_TYPES, null=True) - contract_type = models.PositiveSmallIntegerField(choices=CONTRACT_TYPES, null=True) - picture_url = models.URLField(max_length=250, null=True) picture_entrance_url = models.URLField(max_length=500, null=True) streetview_entrance_url = models.URLField(max_length=500, null=True) @@ -160,6 +125,8 @@ class Unit(SoftDeleteModel): www = models.URLField(max_length=400, null=True) address_postal_full = models.CharField(max_length=100, null=True) call_charge_info = models.CharField(max_length=500, null=True) + displayed_service_owner = models.CharField(max_length=120, null=True) + displayed_service_owner_type = models.CharField(max_length=100, null=True) picture_caption = models.TextField(null=True) diff --git a/services/search/tests/conftest.py b/services/search/tests/conftest.py index 852f585dc..bcd7c839b 100644 --- a/services/search/tests/conftest.py +++ b/services/search/tests/conftest.py @@ -63,7 +63,10 @@ def units( name_en="Biological Museum", street_address="Neitsytpolku 1", municipality=municipality, - contract_type=1, + displayed_service_owner_type="municipal_service", + displayed_service_owner_fi="kunnallinen palvelu", + displayed_service_owner_sv="kommunal tjänst", + displayed_service_owner_en="municipal service", department=department, last_modified_time=now(), location=Point(22.24, 60.44, srid=4326), diff --git a/services/search/tests/test_api.py b/services/search/tests/test_api.py index d24653dee..048fd4652 100644 --- a/services/search/tests/test_api.py +++ b/services/search/tests/test_api.py @@ -34,15 +34,15 @@ def test_search( assert biological_museum_unit["contract_type"]["id"] == "municipal_service" assert ( biological_museum_unit["contract_type"]["description"]["fi"] - == "municipal_service" + == "kunnallinen palvelu" ) assert ( biological_museum_unit["contract_type"]["description"]["sv"] - == "municipal_service" + == "kommunal tjänst" ) assert ( biological_museum_unit["contract_type"]["description"]["en"] - == "municipal_service" + == "municipal service" ) assert biological_museum_unit["department"]["name"]["fi"] == "Test Department" assert ( diff --git a/services/translation.py b/services/translation.py index 0302cd13c..dc62192a1 100644 --- a/services/translation.py +++ b/services/translation.py @@ -58,6 +58,7 @@ class UnitTranslationOptions(TranslationOptions): "picture_caption", "address_postal_full", "call_charge_info", + "displayed_service_owner", ) From d79289bd9de8d018c7e26cc34668b97d62896387 Mon Sep 17 00:00:00 2001 From: juuso-j <68938778+juuso-j@users.noreply.github.com> Date: Mon, 1 Jul 2024 10:47:17 +0300 Subject: [PATCH 2/3] Handle displayed_service_owner Updated with changes made by Helsinki so that tests will pass. --- .../commands/services_import/units.py | 247 ++++++------------ .../commands/services_import/utils.py | 60 ++++- 2 files changed, 145 insertions(+), 162 deletions(-) diff --git a/services/management/commands/services_import/units.py b/services/management/commands/services_import/units.py index 46d038c9b..16891c072 100644 --- a/services/management/commands/services_import/units.py +++ b/services/management/commands/services_import/units.py @@ -28,15 +28,17 @@ UnitIdentifier, UnitServiceDetails, ) -from services.models.unit import ( - CONTRACT_TYPES, - ORGANIZER_TYPES, - PROJECTION_SRID, - PROVIDER_TYPES, -) +from services.models.unit import ORGANIZER_TYPES, PROJECTION_SRID, PROVIDER_TYPES from services.utils import AccessibilityShortcomingCalculator -from .utils import clean_text, pk_get, postcodes, save_translated_field +from .utils import ( + clean_text, + pk_get, + postcodes, + save_translated_field, + update_extra_searchwords, + update_service_names_fields, +) UTC_TIMEZONE = pytz.timezone("UTC") ACTIVE_TIMEZONE = pytz.timezone(settings.TIME_ZONE) @@ -73,105 +75,7 @@ def get_service_ids(): def _fetch_units(): if VERBOSITY: LOGGER.info("Fetching units") - return pk_get("unit", params={"official": "yes"}) - - -CONTRACT_TYPE_MAPPINGS = [ - ("MUNICIPALITY", "SELF_PRODUCED", None, "municipal_service"), - ("MUNICIPALITY", "PURCHASED_SERVICE", None, "purchased_service"), - ("MUNICIPALITY", "VOUCHER_SERVICE", None, "voucher_service"), - ("MUNICIPALITY", "PAYMENT_COMMITMENT", None, "private_service"), - ("MUNICIPALITY", "SUPPORTED_OPERATIONS", None, "supported_operations"), - ("MUNICIPALITY", "CONTRACT_SCHOOL", None, "private_contract_school"), - ( - "MUNICIPALLY_OWNED_COMPANY", - "SELF_PRODUCED", - None, - "service_by_municipally_owned_company", - ), - ( - "MUNICIPAL_ENTERPRISE_GROUP", - "SELF_PRODUCED", - None, - "service_by_municipal_group_entity", - ), - ( - "JOINT_MUNICIPAL_AUTHORITY", - "SELF_PRODUCED", - None, - "service_by_joint_municipal_authority", - ), - ( - "OTHER_REGIONAL_COOPERATION_ORGANIZATION", - "SELF_PRODUCED", - None, - "service_by_regional_cooperation_organization", - ), - ("GOVERNMENT", "SELF_PRODUCED", None, "state_service"), - ("GOVERNMENT", "CONTRACT_SCHOOL", None, "state_contract_school"), - ("GOVERNMENTAL_COMPANY", "SELF_PRODUCED", None, "state_service"), - ("ORGANIZATION", "SELF_PRODUCED", None, "private_service"), - ("ORGANIZATION", "CONTRACT_SCHOOL", None, "private_contract_school"), - ("FOUNDATION", "SELF_PRODUCED", None, "private_service"), - ("FOUNDATION", "CONTRACT_SCHOOL", None, "private_contract_school"), - ("ASSOCIATION", "SELF_PRODUCED", None, "private_service"), - ("ASSOCIATION", "CONTRACT_SCHOOL", None, "private_contract_school"), - ("PRIVATE_ENTERPRISE", "SELF_PRODUCED", None, "private_service"), - ("PRIVATE_ENTERPRISE", "CONTRACT_SCHOOL", None, "private_contract_school"), - ( - "PRIVATE_ENTERPRICE", - "OTHER_PRODUCTION_METHOD", - None, - "private_service", - ), - ( - "MUNICIPALITY", - "OTHER_PRODUCTION_METHOD", - "MUNICIPALITY", - "service_by_other_municipality", - ), - ( - "MUNICIPALITY", - "OTHER_PRODUCTION_METHOD", - "MUNICIPALLY_OWNED_COMPANY", - "service_by_other_municipality", - ), - ( - "MUNICIPALITY", - "OTHER_PRODUCTION_METHOD", - "MUNICIPAL_ENTERPRISE_GROUP", - "service_by_other_municipality", - ), - ( - "MUNICIPALITY", - "OTHER_PRODUCTION_METHOD", - "JOINT_MUNICIPAL_AUTHORITY", - "service_by_joint_municipal_authority", - ), - ( - "MUNICIPALITY", - "OTHER_PRODUCTION_METHOD", - "OTHER_REGIONAL_COOPERATION_ORGANIZATION", - "service_by_regional_cooperation_organization", - ), - ("MUNICIPALITY", "OTHER_PRODUCTION_METHOD", "GOVERNMENT", "state_service"), - ( - "MUNICIPALITY", - "OTHER_PRODUCTION_METHOD", - "GOVERNMENTAL_COMPANY", - "state_service", - ), - ("MUNICIPALITY", "OTHER_PRODUCTION_METHOD", "ORGANIZATION", "private_service"), - ("MUNICIPALITY", "OTHER_PRODUCTION_METHOD", "FOUNDATION", "private_service"), - ("MUNICIPALITY", "OTHER_PRODUCTION_METHOD", "ASSOCIATION", "private_service"), - ( - "MUNICIPALITY", - "OTHER_PRODUCTION_METHOD", - "PRIVATE_ENTERPRISE", - "private_service", - ), - ("MUNICIPALITY", "OTHER_PRODUCTION_METHOD", "UNKNOWN", "private_service"), -] + return pk_get("unit", params={"official": "yes", "newfeatures": "yes"}) def import_units( @@ -239,7 +143,11 @@ def import_units( if fetch_only_id: obj_id = fetch_only_id - obj_list = [fetch_resource("unit", obj_id, params={"official": "yes"})] + obj_list = [ + fetch_resource( + "unit", obj_id, params={"official": "yes", "newfeatures": "yes"} + ) + ] queryset = Unit.objects.filter(id=obj_id) else: obj_list = fetch_units() @@ -319,6 +227,7 @@ def _import_unit( "picture_caption", "address_postal_full", "call_charge_info", + "displayed_service_owner", ) for field in fields_that_need_translation: if save_translated_field(obj, field, info, field): @@ -336,41 +245,57 @@ def _import_unit( LOGGER.warning("%s: coordinates present but no city" % obj) municipality_id = None - muni_name = info.get("address_city_fi", None) - if not muni_name and "address_zip" in info: - muni_name = "no-city" - if muni_name: - muni_name = muni_name.lower() - if muni_name in ("helsingin kaupunki",): - muni_name = "helsinki" - elif muni_name in ("vantaan kaupunki",): - muni_name = "vantaa" - elif muni_name in ("espoon kaupunki",): - muni_name = "espoo" - if muni_name not in muni_by_name: - postcode = info.get("address_zip", None) - muni_name = postcodes().get(postcode, None) - if muni_name: - if VERBOSITY: - LOGGER.warning( - "%s: municipality to %s based on post code %s (was %s)" - % (obj, muni_name, postcode, info.get("address_city_fi")) - ) - muni_name = muni_name.lower() + muni_name = None + org_id = info.get("org_id", None) + + # FIXME: Temporarily skip health district units completely + if org_id == "5de91045-92ab-484b-9f96-7010ff7fb35e": + LOGGER.info("Temporarily skipping health district unit: %s" % obj) + return None + + if org_id: + department_qs = Department.objects.filter(uuid=org_id) + if department_qs: + municipality = department_qs.first().municipality + if municipality: + muni_name = municipality.id + + if not muni_name: + muni_name = info.get("address_city_fi", None) + if not muni_name and "address_zip" in info: + muni_name = "no-city" if muni_name: - muni = muni_by_name.get(muni_name) - if muni: - municipality_id = muni.id - else: - if VERBOSITY: - LOGGER.warning( - "%s: municipality %s not found from current Municipalities" - % (obj, muni_name) - ) + muni_name = muni_name.lower() + if muni_name in ("helsingin kaupunki",): + muni_name = "helsinki" + elif muni_name in ("vantaan kaupunki",): + muni_name = "vantaa" + elif muni_name in ("espoon kaupunki",): + muni_name = "espoo" + if muni_name not in muni_by_name: + postcode = info.get("address_zip", None) + muni_name = postcodes().get(postcode, None) + if muni_name: + if VERBOSITY: + LOGGER.warning( + "%s: municipality to %s based on post code %s (was %s)" + % (obj, muni_name, postcode, info.get("address_city_fi")) + ) + muni_name = muni_name.lower() + if muni_name: + muni = muni_by_name.get(muni_name) + if muni: + municipality_id = muni.id + else: + if VERBOSITY: + LOGGER.warning( + "%s: municipality %s not found from current Municipalities" + % (obj, muni_name) + ) - if municipality_id: - # self._set_field(obj, 'municipality_id', municipality_id) + if municipality_id and municipality_id != obj.municipality_id: obj.municipality_id = municipality_id + obj_changed = True dept = None dept_id = None @@ -408,27 +333,11 @@ def _import_unit( "streetview_entrance_url", "organizer_name", "organizer_business_id", + "displayed_service_owner_type", + "vtj_prt", + "vtj_prt_verified", ] - contract_type = None - if dept: - organization_type = dept.organization_type - for mapping in CONTRACT_TYPE_MAPPINGS: - if mapping[0] != organization_type: - continue - if mapping[1] != info.get("provider_type"): - continue - if mapping[2] in [None, info.get("organizer_type")]: - contract_type = mapping[3] - break - if contract_type: - ctype = next( - (val for val, str_val in CONTRACT_TYPES if str_val == contract_type) - ) - if obj.contract_type != ctype: - obj.contract_type = ctype - obj_changed = True - if info.get("provider_type"): info["provider_type"] = [ val for val, str_val in PROVIDER_TYPES if str_val == info["provider_type"] @@ -525,6 +434,12 @@ def _import_unit( obj_changed = True obj.public = is_public + # if unit was previously soft-deleted, make it active again + if not obj.is_active: + obj_changed = True + obj.is_active = True + obj.deleted_at = None + maintenance_organization = muni_name if obj.extensions is None: obj.extensions = {} @@ -558,8 +473,13 @@ def _import_unit( obj_changed, update_fields = _import_unit_services( obj, info, obj_changed, update_fields ) + obj_changed, update_fields = update_service_names_fields( + obj, info, obj_changed, update_fields + ) obj_changed = keyword_handler.sync_searchwords(obj, info, obj_changed) - + obj_changed, update_fields = update_extra_searchwords( + obj, info, obj_changed, update_fields + ) obj_changed, update_fields = _import_unit_accessibility_variables( obj, info, obj_changed, update_fields ) @@ -649,6 +569,7 @@ def _import_unit_services(obj, info, obj_changed, update_fields): unit_owd.save() obj.service_details_hash = owd_hash + obj_changed = True update_fields.append("service_details_hash") @@ -719,7 +640,7 @@ def _import_unit_connections(obj, info, obj_changed, update_fields): for i, conn in enumerate(info["connections"]): c = UnitConnection(unit=obj) - save_translated_field(c, "name", conn, "name", max_length=600) + save_translated_field(c, "name", conn, "name", max_length=2100) save_translated_field(c, "www", conn, "www") section_type = [ val @@ -730,6 +651,12 @@ def _import_unit_connections(obj, info, obj_changed, update_fields): c.section_type = section_type c.order = i + + tags = conn.get("tags", []) + if tags and getattr(c, "tags") != tags: + setattr(c, "tags", tags) + c._changed = True + fields = ["email", "phone", "contact_person"] for field in fields: val = conn.get(field, None) diff --git a/services/management/commands/services_import/utils.py b/services/management/commands/services_import/utils.py index 94cf72add..a9e141577 100644 --- a/services/management/commands/services_import/utils.py +++ b/services/management/commands/services_import/utils.py @@ -29,8 +29,6 @@ def save_translated_field(obj, obj_field_name, info, info_field_name, max_length else: val = None if max_length and val and len(val) > max_length: - # if self.verbosity: - # self.logger.warning("%s: field '%s' too long" % (obj, obj_field_name)) val = None obj_key = "%s_%s" % (obj_field_name, lang) obj_val = getattr(obj, obj_key) @@ -62,6 +60,64 @@ def clean_text(text): return text +def update_service_names_fields(obj, info, obj_changed, update_fields): + service_names_fi = [] + service_names_sv = [] + service_names_en = [] + for service in obj.services.all(): + service_names_fi.append(service.name_fi) + service_names_sv.append(service.name_sv) + service_names_en.append(service.name_en) + + if ( + obj.service_names_fi == service_names_fi + and obj.service_names_sv == service_names_sv + and obj.service_names_en == service_names_en + ): + return False, update_fields + + setattr(obj, "service_names_fi", service_names_fi) + setattr(obj, "service_names_sv", service_names_sv) + setattr(obj, "service_names_en", service_names_en) + update_fields.extend(["service_names_fi", "service_names_sv", "service_names_en"]) + return True, update_fields + + +def convert_to_list(text): + return [e.strip() for e in text.split(",")] + + +def get_extra_searchwords(info, language): + field_name = "extra_searchwords_%s" % language + val = info.get(field_name, []) + if val: + val = convert_to_list(val) + return val + + +def update_extra_searchwords(obj, info, obj_changed, update_fields): + extra_searchwords_fi = get_extra_searchwords(info, "fi") + extra_searchwords_sv = get_extra_searchwords(info, "sv") + extra_searchwords_en = get_extra_searchwords(info, "en") + if ( + obj.extra_searchwords_fi == extra_searchwords_fi + and obj.extra_searchwords_sv == extra_searchwords_sv + and obj.extra_searchwords_en == extra_searchwords_en + ): + return False, update_fields + + if extra_searchwords_fi: + setattr(obj, "extra_searchwords_fi", extra_searchwords_fi) + update_fields.append("extra_searchwords_fi") + if extra_searchwords_sv: + setattr(obj, "extra_searchwords_sv", extra_searchwords_sv) + update_fields.append("extra_searchwords_sv") + if extra_searchwords_en: + setattr(obj, "extra_searchwords_en", extra_searchwords_en) + update_fields.append("extra_searchwords_en") + return True, update_fields + + def postcodes(): path = os.path.join(settings.BASE_DIR, "data", "fi", "postcodes.txt") postcodes = {} From 1bb90c21ad379449669875485b89278a7b5e5681 Mon Sep 17 00:00:00 2001 From: juuso-j <68938778+juuso-j@users.noreply.github.com> Date: Mon, 1 Jul 2024 12:18:39 +0300 Subject: [PATCH 3/3] Expand no_private_services filter to exclude "not displayed" services --- services/api.py | 1 + 1 file changed, 1 insertion(+) diff --git a/services/api.py b/services/api.py index 5020b425f..ca696d601 100644 --- a/services/api.py +++ b/services/api.py @@ -1035,6 +1035,7 @@ def validate_service_node_ids(service_node_ids): ) queryset = queryset.exclude( Q(displayed_service_owner_type__iexact="PRIVATE_SERVICE") + | Q(displayed_service_owner_type__iexact="NOT_DISPLAYED") | Q(organizer_type=private_enterprise_value) )