From 5c48682bf38c63d0d221f13d0873c5ea1411eaf9 Mon Sep 17 00:00:00 2001 From: Niko Lindroos Date: Mon, 23 Feb 2026 12:16:06 +0200 Subject: [PATCH] feat(ks,api): remove referencable employment fields from API YJDH-811. Some fields can be referenced through the link from EmployerSummerVoucher to YouthSummerVoucher, meaning that these values are already filled by youth, when they were requested from employer. Employer does not know and might not care about the data that should be added to those fields and they really are fields related to youth, not the employment, so they should not be asked from employer. --- backend/kesaseteli/applications/admin.py | 6 +- .../applications/api/v1/serializers.py | 10 +- ...mmervoucher_employee_home_city_and_more.py | 53 ++++++ backend/kesaseteli/applications/models.py | 111 +++++++---- .../test_employer_summer_voucher_admin.py | 18 +- .../applications/tests/test_excel_export.py | 11 +- .../test_serial_number_migration_functions.py | 180 +----------------- backend/kesaseteli/common/tests/factories.py | 5 - 8 files changed, 155 insertions(+), 239 deletions(-) create mode 100644 backend/kesaseteli/applications/migrations/0049_remove_employersummervoucher_employee_home_city_and_more.py diff --git a/backend/kesaseteli/applications/admin.py b/backend/kesaseteli/applications/admin.py index 09eb70d0b8..26d5d96e5b 100644 --- a/backend/kesaseteli/applications/admin.py +++ b/backend/kesaseteli/applications/admin.py @@ -630,10 +630,14 @@ class EmployerSummerVoucherAdmin(admin.ModelAdmin): "target_group_display", "application", "masked_employee_ssn", + "employee_name", + "employee_school", + "employee_home_city", + "employee_postcode", "created_at", "modified_at", ] - exclude = ["employee_ssn"] + exclude = [] # custom property to list employee related fields. employee_fields = [ diff --git a/backend/kesaseteli/applications/api/v1/serializers.py b/backend/kesaseteli/applications/api/v1/serializers.py index 215da9f3ef..1712e2bbe9 100644 --- a/backend/kesaseteli/applications/api/v1/serializers.py +++ b/backend/kesaseteli/applications/api/v1/serializers.py @@ -254,6 +254,11 @@ class Meta: ] read_only_fields = [ "ordering", + "employee_name", + "employee_school", + "employee_ssn", + "employee_home_city", + "employee_postcode", ] list_serializer_class = EmployerSummerVoucherListSerializer @@ -319,12 +324,7 @@ def validate(self, data): REQUIRED_FIELDS_FOR_SUBMITTED_SUMMER_VOUCHERS = [ "summer_voucher_serial_number", - "employee_name", - "employee_school", - "employee_ssn", "employee_phone_number", - "employee_home_city", - "employee_postcode", "employment_postcode", "employment_start_date", "employment_end_date", diff --git a/backend/kesaseteli/applications/migrations/0049_remove_employersummervoucher_employee_home_city_and_more.py b/backend/kesaseteli/applications/migrations/0049_remove_employersummervoucher_employee_home_city_and_more.py new file mode 100644 index 0000000000..edc6e21cb5 --- /dev/null +++ b/backend/kesaseteli/applications/migrations/0049_remove_employersummervoucher_employee_home_city_and_more.py @@ -0,0 +1,53 @@ +# Generated by Django 5.1.15 on 2026-02-23 10:35 + +from django.db import migrations + + +class Migration(migrations.Migration): + + dependencies = [ + ('applications', '0048_employerapplication_add_timestamp_and_status_indexes'), + ] + + operations = [ + migrations.RemoveField( + model_name='employersummervoucher', + name='employee_home_city', + ), + migrations.RemoveField( + model_name='employersummervoucher', + name='employee_name', + ), + migrations.RemoveField( + model_name='employersummervoucher', + name='employee_postcode', + ), + migrations.RemoveField( + model_name='employersummervoucher', + name='employee_school', + ), + migrations.RemoveField( + model_name='employersummervoucher', + name='employee_ssn', + ), + migrations.RemoveField( + model_name='historicalemployersummervoucher', + name='employee_home_city', + ), + migrations.RemoveField( + model_name='historicalemployersummervoucher', + name='employee_name', + ), + migrations.RemoveField( + model_name='historicalemployersummervoucher', + name='employee_postcode', + ), + migrations.RemoveField( + model_name='historicalemployersummervoucher', + name='employee_school', + ), + migrations.RemoveField( + model_name='historicalemployersummervoucher', + name='employee_ssn', + ), + ] diff --git a/backend/kesaseteli/applications/models.py b/backend/kesaseteli/applications/models.py index a86435a390..e99f719256 100644 --- a/backend/kesaseteli/applications/models.py +++ b/backend/kesaseteli/applications/models.py @@ -1265,7 +1265,23 @@ class Meta: ] +class EmployerSummerVoucherQuerySet(models.QuerySet): + def select_youth_voucher(self): + return self.select_related( + "youth_summer_voucher", "youth_summer_voucher__youth_application" + ) + + +class EmployerSummerVoucherManager(models.Manager): + def get_queryset(self): + return EmployerSummerVoucherQuerySet( + self.model, using=self._db + ).select_youth_voucher() + + class EmployerSummerVoucher(HistoricalModel, TimeStampedModel, UUIDModel): + objects = EmployerSummerVoucherManager() + application = models.ForeignKey( EmployerApplication, on_delete=models.CASCADE, @@ -1298,55 +1314,51 @@ class EmployerSummerVoucher(HistoricalModel, TimeStampedModel, UUIDModel): ), ) + employee_phone_number = models.CharField( + max_length=64, + blank=True, + verbose_name=_("employee phone number"), + ) + @property - def target_group(self): - """ - Get the target group of the youth summer voucher. - NOTE: This is mostly for backward compatibility with the old target_group field. - """ + def employee_name(self) -> str: return ( - self.youth_summer_voucher.target_group if self.youth_summer_voucher else "" + self.youth_summer_voucher.youth_application.name + if self.youth_summer_voucher + else "" ) - # Mimicking Django's standard API. ChoiceField's magic is gone - # since target_group is now migrated to a custom property. - def get_target_group_display(self): + @property + def employee_school(self) -> str: return ( - self.youth_summer_voucher.get_target_group_display() + self.youth_summer_voucher.youth_application.school if self.youth_summer_voucher else "" ) - employee_name = models.CharField( - max_length=256, - blank=True, - verbose_name=_("employee name"), - ) - employee_school = models.CharField( - max_length=256, - blank=True, - verbose_name=_("employee school"), - ) - employee_ssn = EncryptedCharField( - max_length=32, - blank=True, - verbose_name=_("employee social security number"), - ) - employee_phone_number = models.CharField( - max_length=64, - blank=True, - verbose_name=_("employee phone number"), - ) - employee_home_city = models.CharField( - max_length=256, - blank=True, - verbose_name=_("employee home city"), - ) - employee_postcode = models.CharField( - max_length=256, - blank=True, - verbose_name=_("employee postcode"), - ) + @property + def employee_ssn(self) -> str: + return ( + self.youth_summer_voucher.youth_application.social_security_number + if self.youth_summer_voucher + else "" + ) + + @property + def employee_home_city(self) -> str: + return ( + self.youth_summer_voucher.youth_application.home_municipality + if self.youth_summer_voucher + else "" + ) + + @property + def employee_postcode(self) -> str: + return ( + self.youth_summer_voucher.youth_application.postcode + if self.youth_summer_voucher + else "" + ) employment_postcode = models.CharField( max_length=256, @@ -1401,6 +1413,25 @@ def get_target_group_display(self): ordering = models.IntegerField(default=0) + @property + def target_group(self): + """ + Get the target group of the youth summer voucher. + NOTE: This is mostly for backward compatibility with the old target_group field. + """ + return ( + self.youth_summer_voucher.target_group if self.youth_summer_voucher else "" + ) + + # Mimicking Django's standard API. ChoiceField's magic is gone + # since target_group is now migrated to a custom property. + def get_target_group_display(self): + return ( + self.youth_summer_voucher.get_target_group_display() + if self.youth_summer_voucher + else "" + ) + @property def summer_voucher_serial_number(self) -> str: """ diff --git a/backend/kesaseteli/applications/tests/test_employer_summer_voucher_admin.py b/backend/kesaseteli/applications/tests/test_employer_summer_voucher_admin.py index 266d88d80d..5fd8d15a7b 100644 --- a/backend/kesaseteli/applications/tests/test_employer_summer_voucher_admin.py +++ b/backend/kesaseteli/applications/tests/test_employer_summer_voucher_admin.py @@ -14,29 +14,37 @@ def employer_summer_voucher_admin(): @pytest.mark.django_db def test_masked_employee_ssn(employer_summer_voucher_admin): # Test with valid SSN - voucher = EmployerSummerVoucherFactory.build(employee_ssn="010101-1234") + voucher = EmployerSummerVoucherFactory.build( + youth_summer_voucher__youth_application__social_security_number="010101-1234" + ) assert employer_summer_voucher_admin.masked_employee_ssn(voucher) == "******1234" # Test with another valid SSN - voucher = EmployerSummerVoucherFactory.build(employee_ssn="311299A9876") + voucher = EmployerSummerVoucherFactory.build( + youth_summer_voucher__youth_application__social_security_number="311299A9876" + ) assert employer_summer_voucher_admin.masked_employee_ssn(voucher) == "******9876" @pytest.mark.django_db def test_masked_employee_ssn_empty(employer_summer_voucher_admin): # Test with empty SSN - voucher = EmployerSummerVoucherFactory.build(employee_ssn="") + voucher = EmployerSummerVoucherFactory.build( + youth_summer_voucher__youth_application__social_security_number="" + ) assert employer_summer_voucher_admin.masked_employee_ssn(voucher) == "" # Test with None SSN - voucher = EmployerSummerVoucherFactory.build(employee_ssn=None) + voucher = EmployerSummerVoucherFactory.build(youth_summer_voucher=None) assert employer_summer_voucher_admin.masked_employee_ssn(voucher) == "" @pytest.mark.django_db def test_masked_employee_ssn_short(employer_summer_voucher_admin): # Test with short SSN - voucher = EmployerSummerVoucherFactory.build(employee_ssn="123") + voucher = EmployerSummerVoucherFactory.build( + youth_summer_voucher__youth_application__social_security_number="123" + ) assert employer_summer_voucher_admin.masked_employee_ssn(voucher) == "******123" diff --git a/backend/kesaseteli/applications/tests/test_excel_export.py b/backend/kesaseteli/applications/tests/test_excel_export.py index a91fe24d13..8ba6a57288 100644 --- a/backend/kesaseteli/applications/tests/test_excel_export.py +++ b/backend/kesaseteli/applications/tests/test_excel_export.py @@ -56,6 +56,7 @@ YouthApplicationFactory, ) from common.urls import handler_403_url +from common.utils import getattr_nested from shared.audit_log.models import AuditLogEntry @@ -352,11 +353,13 @@ def employer_summer_voucher_sorting_key(voucher: EmployerSummerVoucher): elif excel_field.model_fields == []: assert output_column.value == excel_field.value else: - query = EmployerSummerVoucher.objects.filter(pk=voucher.pk) - values_tuple = query.values_list(*excel_field.model_fields)[0] - assert output_column.value == excel_field.value % values_tuple, ( - excel_field.title + values_tuple = tuple( + getattr_nested(voucher, attr_str.split("__")) + for attr_str in excel_field.model_fields ) + assert str(output_column.value) == str( + excel_field.value % values_tuple + ), excel_field.title @pytest.mark.django_db diff --git a/backend/kesaseteli/applications/tests/test_serial_number_migration_functions.py b/backend/kesaseteli/applications/tests/test_serial_number_migration_functions.py index 7a503aeaea..a04a70577f 100644 --- a/backend/kesaseteli/applications/tests/test_serial_number_migration_functions.py +++ b/backend/kesaseteli/applications/tests/test_serial_number_migration_functions.py @@ -8,10 +8,7 @@ maintain them indefinitely. """ -from datetime import datetime - import pytest -from django.utils import timezone from applications.migrations.helpers.serial_number_foreign_keys import ( set_current_valid_serial_number_based_foreign_keys, @@ -30,10 +27,7 @@ def create_employer_summer_voucher(**overrides): """Create EmployerSummerVoucher with test defaults and the given overrides.""" params = { "youth_summer_voucher": None, - "employee_name": "Test Employee", "employee_phone_number": "+3584012345678", - "employee_home_city": "Helsinki", - "employee_postcode": "00100", "employment_postcode": "00200", "employment_start_date": "2024-06-01", "employment_end_date": "2024-08-31", @@ -70,48 +64,6 @@ def employer_app(): ) -@pytest.fixture -def complex_data_setup(employer_app): - """ - Create a complex set of EmployerSummerVoucher and YouthSummerVoucher - objects to be used in tests for set_current_valid_serial_number_based_foreign_keys. - """ - year = 2025 - - # Create 11 YouthSummerVouchers that should match by serial number - for serial_number in range(1, 12): - YouthSummerVoucherFactory(summer_voucher_serial_number=serial_number) - create_employer_summer_voucher( - application=employer_app, - _obsolete_unclean_serial_number=str(serial_number), - employee_ssn="111111-111C", - ) - - # Create 12 YouthSummerVouchers that should match by SSN - for serial_number in range(12, 24): - youth_voucher = YouthSummerVoucherFactory( - summer_voucher_serial_number=serial_number - ) - youth_app = youth_voucher.youth_application - youth_app.created_at = timezone.make_aware(datetime(year, 5, 1)) - youth_app.save() - - create_employer_summer_voucher( - application=employer_app, - _obsolete_unclean_serial_number="INVALID", # Won't match by serial - employee_ssn=youth_app.social_security_number, - created_at=timezone.make_aware(datetime(year, 6, 15)), - ) - - # Create 7 EmployerSummerVouchers that shouldn't match anything - for _ in range(7): - create_employer_summer_voucher( - application=employer_app, - _obsolete_unclean_serial_number="NOMATCH", - employee_ssn="INVALID", - ) - - @pytest.mark.django_db def test_match_by_numeric_serial_number(employer_app): """ @@ -125,7 +77,6 @@ def test_match_by_numeric_serial_number(employer_app): employer_voucher = create_employer_summer_voucher( application=employer_app, _obsolete_unclean_serial_number=" 00012345 ", - employee_ssn="121212A899H", ) # Before migration, foreign key should be None @@ -143,134 +94,6 @@ def test_match_by_numeric_serial_number(employer_app): assert employer_voucher.youth_summer_voucher.summer_voucher_serial_number == 12345 -@pytest.mark.django_db -@pytest.mark.parametrize("youth_app_created_before_employer_app", [True, False]) -def test_match_by_ssn_and_year(youth_app_created_before_employer_app, employer_app): - """ - Test the fallback matching in set_current_valid_serial_number_based_foreign_keys - by social security number and creation year when the serial number does not match. - - :param youth_app_created_before_employer_app: Was the youth application created - before the employer application? - """ - # Create a YouthSummerVoucher - youth_voucher = YouthSummerVoucherFactory(summer_voucher_serial_number=99999) - youth_app = youth_voucher.youth_application - created_year = youth_app.created_at.year - - # Create an EmployerSummerVoucher with non-numeric serial but matching SSN - employer_voucher = create_employer_summer_voucher( - application=employer_app, - _obsolete_unclean_serial_number="ABC123", # Non-numeric → won't match by serial - employee_ssn=youth_app.social_security_number, - created_at=timezone.make_aware(datetime(created_year, 6, 15)), - ) - - # Ensure youth application was created before/after employer voucher in the same year - youth_app.created_at = timezone.make_aware( - datetime(created_year, 5 if youth_app_created_before_employer_app else 7, 1) - ) - youth_app.save() - - # Before migration, foreign key should be None - assert employer_voucher.youth_summer_voucher is None - - # Run the migration function - set_current_valid_serial_number_based_foreign_keys( - EmployerSummerVoucher, YouthSummerVoucher - ) - - # Verify the result - employer_voucher.refresh_from_db() - assert ( - employer_voucher.youth_summer_voucher is not None - ) == youth_app_created_before_employer_app - assert ( - employer_voucher.youth_summer_voucher_id - == youth_voucher.summer_voucher_serial_number - ) == youth_app_created_before_employer_app - - -@pytest.mark.django_db -def test_ambiguous_multimatch_with_ssn(employer_app): - """ - Test that no match will be found in - set_current_valid_serial_number_based_foreign_keys when serial number does - not match, and the fallback matching using social security number matches - multiple youth summer vouchers. - - The foreign key should remain None due to ambiguity. - """ - # Create two YouthSummerVouchers with the same social security number - # and application year - ssn = "010203-1230" - created_at = timezone.make_aware(datetime(2024, 5, 1)) - - for serial_number in 11111, 22222: - youth_voucher = YouthSummerVoucherFactory( - summer_voucher_serial_number=serial_number, - youth_application__social_security_number=ssn, - ) - youth_app = youth_voucher.youth_application - youth_app.created_at = created_at - youth_app.save() - - # Create an EmployerSummerVoucher with matching social security number - employer_voucher = create_employer_summer_voucher( - application=employer_app, - _obsolete_unclean_serial_number="ABC123", # Invalid serial number - employee_ssn=ssn, - created_at=timezone.make_aware(datetime(2024, 6, 1)), - ) - - # Before migration, foreign key should be None - assert employer_voucher.youth_summer_voucher is None - - # Run the migration function - set_current_valid_serial_number_based_foreign_keys( - EmployerSummerVoucher, YouthSummerVoucher - ) - - # After migration, foreign key should still be None (ambiguous match) - employer_voucher.refresh_from_db() - assert employer_voucher.youth_summer_voucher is None - - -@pytest.mark.django_db -def test_set_current_valid_serial_number_query_count( - complex_data_setup, django_assert_max_num_queries -): - """ - Test that set_current_valid_serial_number_based_foreign_keys - uses at most 3 queries with the data set up in `complex_data_setup` - fixture (it should be 30 EmployerSummerVoucher objects) to process. - - NOTE: - With larger datasets, the chunk/bulk sizes in the migration - function will make the actual query count higher! - """ - with django_assert_max_num_queries(3): - set_current_valid_serial_number_based_foreign_keys( - EmployerSummerVoucher, YouthSummerVoucher - ) - - -@pytest.mark.django_db -def test_set_current_valid_serial_number_logger_output(complex_data_setup, caplog): - """ - Test that set_current_valid_serial_number_based_foreign_keys - logs the expected summary output. - """ - with caplog.at_level("INFO"): - set_current_valid_serial_number_based_foreign_keys( - EmployerSummerVoucher, YouthSummerVoucher - ) - assert "Handled 30 employer summer vouchers, updated 23:\n" in caplog.text - assert "- Matched by voucher serial number: 11\n" in caplog.text - assert "- Matched by social security number & year: 12\n" in caplog.text - assert "- Failed to match: 7 and left as is\n" in caplog.text - - @pytest.mark.django_db @pytest.mark.parametrize( "obsolete_serial,expected_id", @@ -298,7 +121,6 @@ def test_set_historical_serial_number_based_foreign_keys( employer_voucher = create_employer_summer_voucher( application=employer_app, _obsolete_unclean_serial_number=obsolete_serial, - employee_ssn="111111-111C", ) # HistoricalEmployerSummerVoucher does exist, not all IDEs (e.g. PyCharm) @@ -314,7 +136,7 @@ def test_set_historical_serial_number_based_foreign_keys( assert HistoricalEmployerSummerVoucher.objects.count() == 0 # Update the object to create historical record - employer_voucher.employee_name = "Updated Name" + employer_voucher.employment_postcode = "00300" employer_voucher.save() assert HistoricalEmployerSummerVoucher.objects.count() == 1 diff --git a/backend/kesaseteli/common/tests/factories.py b/backend/kesaseteli/common/tests/factories.py index 6d8e95a712..2b025f323e 100644 --- a/backend/kesaseteli/common/tests/factories.py +++ b/backend/kesaseteli/common/tests/factories.py @@ -83,12 +83,7 @@ class EmployerSummerVoucherFactory( "common.tests.factories.YouthSummerVoucherFactory" ) - employee_name = factory.Faker("name") - employee_school = factory.Faker("lexify", text="????? School") - employee_ssn = factory.Faker("bothify", text="######-###?") employee_phone_number = factory.Faker("phone_number") - employee_home_city = factory.Faker("city") - employee_postcode = factory.Faker("postcode") employment_postcode = factory.Faker("postcode") employment_start_date = factory.Faker(