From 5c777e8dd9b3bdf804bf24c6eb31bb319f3a0ec6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Romain=20S=C3=A9bille?= Date: Mon, 30 Dec 2024 19:10:32 +0100 Subject: [PATCH] employee_record.models: Use keyword only for transitions' arguments This will be needed to easily log transitions. --- .../commands/transfer_employee_records.py | 14 +++-- .../transfer_employee_records_updates.py | 4 +- itou/employee_record/models.py | 12 ++-- tests/api/employee_record_api/tests.py | 12 ++-- tests/employee_record/test_models.py | 60 +++++++++---------- .../www/employee_record_views/test_disable.py | 10 ++-- tests/www/employee_record_views/test_list.py | 10 ++-- .../employee_record_views/test_reactivate.py | 4 +- .../www/employee_record_views/test_summary.py | 2 +- 9 files changed, 67 insertions(+), 61 deletions(-) diff --git a/itou/employee_record/management/commands/transfer_employee_records.py b/itou/employee_record/management/commands/transfer_employee_records.py index f6d068805d..2387255520 100644 --- a/itou/employee_record/management/commands/transfer_employee_records.py +++ b/itou/employee_record/management/commands/transfer_employee_records.py @@ -56,7 +56,11 @@ def _upload_batch_file(self, sftp: paramiko.SFTPClient, employee_records: list[E # and store in which file they have been sent renderer = JSONRenderer() for idx, employee_record in enumerate(employee_records, 1): - employee_record.sent(remote_path, idx, renderer.render(batch_data["lignesTelechargement"][idx - 1])) + employee_record.sent( + file=remote_path, + line_number=idx, + archive=renderer.render(batch_data["lignesTelechargement"][idx - 1]), + ) def _parse_feedback_file(self, feedback_file: str, batch: dict, dry_run: bool) -> None: """ @@ -95,7 +99,7 @@ def _parse_feedback_file(self, feedback_file: str, batch: dict, dry_run: bool) - archived_json = JSONRenderer().render(raw_employee_record) if processing_code == EmployeeRecord.ASP_PROCESSING_SUCCESS_CODE: # Processed by ASP if not dry_run: - employee_record.process(processing_code, processing_label, archived_json) + employee_record.process(code=processing_code, label=processing_label, archive=archived_json) else: self.stdout.write(f"DRY-RUN: Accepted {employee_record=}, {processing_code=}, {processing_label=}") else: # Rejected by ASP @@ -103,7 +107,9 @@ def _parse_feedback_file(self, feedback_file: str, batch: dict, dry_run: bool) - # One special case added for support concerns: # 3436 processing code are automatically converted as PROCESSED if processing_code == EmployeeRecord.ASP_DUPLICATE_ERROR_CODE: - employee_record.process(processing_code, processing_label, archived_json, as_duplicate=True) + employee_record.process( + code=processing_code, label=processing_label, archive=archived_json, as_duplicate=True + ) # If the ASP mark the employee record as duplicate, # and there is a suspension or a prolongation for the associated approval, @@ -123,7 +129,7 @@ def _parse_feedback_file(self, feedback_file: str, batch: dict, dry_run: bool) - continue - employee_record.reject(processing_code, processing_label, archived_json) + employee_record.reject(code=processing_code, label=processing_label, archive=archived_json) else: self.stdout.write(f"DRY-RUN: Rejected {employee_record=}, {processing_code=}, {processing_label=}") diff --git a/itou/employee_record/management/commands/transfer_employee_records_updates.py b/itou/employee_record/management/commands/transfer_employee_records_updates.py index 6e7265350b..ad770b9639 100644 --- a/itou/employee_record/management/commands/transfer_employee_records_updates.py +++ b/itou/employee_record/management/commands/transfer_employee_records_updates.py @@ -100,12 +100,12 @@ def _parse_feedback_file(self, feedback_file: str, batch: dict, dry_run: bool) - archived_json = JSONRenderer().render(employee_record) if processing_code == EmployeeRecordUpdateNotification.ASP_PROCESSING_SUCCESS_CODE: # Processed by ASP if not dry_run: - notification.process(processing_code, processing_label, archived_json) + notification.process(code=processing_code, label=processing_label, archive=archived_json) else: self.stdout.write(f"DRY-RUN: Processed {notification}, {processing_code=}, {processing_label=}") else: # Rejected by ASP if not dry_run: - notification.reject(processing_code, processing_label, archived_json) + notification.reject(code=processing_code, label=processing_label, archive=archived_json) else: self.stdout.write(f"DRY-RUN: Rejected {notification}: {processing_code=}, {processing_label=}") diff --git a/itou/employee_record/models.py b/itou/employee_record/models.py index cf5cd12ae0..b94e777595 100644 --- a/itou/employee_record/models.py +++ b/itou/employee_record/models.py @@ -311,7 +311,7 @@ def ready(self): self._fill_denormalized_fields() @xwf_models.transition() - def sent(self, file, line_number, archive): + def sent(self, *, file, line_number, archive): """ An employee record is sent to ASP via a JSON file, The file name is stored for further feedback processing (also done via a file) @@ -320,7 +320,7 @@ def sent(self, file, line_number, archive): self.set_asp_batch_information(file, line_number, archive) @xwf_models.transition() - def reject(self, code, label, archive): + def reject(self, *, code, label, archive): """ Update status after an ASP rejection of the employee record """ @@ -328,7 +328,7 @@ def reject(self, code, label, archive): self.set_asp_processing_information(code, label, archive) @xwf_models.transition(EmployeeRecordTransition.PROCESS) - def process(self, code, label, archive, *, as_duplicate=False): + def process(self, *, code, label, archive, as_duplicate=False): if as_duplicate and code != self.ASP_DUPLICATE_ERROR_CODE: raise ValueError(f"Code needs to be {self.ASP_DUPLICATE_ERROR_CODE} and not {code} when {as_duplicate=}") @@ -585,13 +585,13 @@ def __repr__(self): return f"<{type(self).__name__} pk={self.pk}>" @xwf_models.transition(EmployeeRecordTransition.SENT) - def sent(self, file, line_number, archive): + def sent(self, *, file, line_number, archive): self.set_asp_batch_information(file, line_number, archive) @xwf_models.transition(EmployeeRecordTransition.REJECT) - def reject(self, code, label, archive): + def reject(self, *, code, label, archive): self.set_asp_processing_information(code, label, archive) @xwf_models.transition(EmployeeRecordTransition.PROCESS) - def process(self, code, label, archive): + def process(self, *, code, label, archive): self.set_asp_processing_information(code, label, archive) diff --git a/tests/api/employee_record_api/tests.py b/tests/api/employee_record_api/tests.py index 79eb6e5f7d..9125b7a5f5 100644 --- a/tests/api/employee_record_api/tests.py +++ b/tests/api/employee_record_api/tests.py @@ -101,7 +101,7 @@ def test_fetch_employee_record_list(self, api_client, mocker, faker): # Get list without filtering by status (PROCESSED) # note: there is no way to create a processed employee record # (and this is perfectly normal) - self.employee_record.sent(faker.asp_batch_filename(), 1, None) + self.employee_record.sent(file=faker.asp_batch_filename(), line_number=1, archive=None) process_code, process_message = "0000", "La ligne de la fiche salarié a été enregistrée avec succès." # There should be no result at this point @@ -111,7 +111,7 @@ def test_fetch_employee_record_list(self, api_client, mocker, faker): result = response.json() assert len(result.get("results")) == 0 - self.employee_record.process(process_code, process_message, "{}") + self.employee_record.process(code=process_code, label=process_message, archive="{}") response = api_client.get(ENDPOINT_URL, format="json") assert response.status_code == 200 @@ -131,7 +131,7 @@ def test_fetch_employee_record_list(self, api_client, mocker, faker): result = response.json() assert len(result.get("results")) == 0 - employee_record_sent.sent(faker.asp_batch_filename(), 1, None) + employee_record_sent.sent(file=faker.asp_batch_filename(), line_number=1, archive=None) response = api_client.get(ENDPOINT_URL + "?status=SENT", format="json") assert response.status_code == 200 @@ -143,7 +143,7 @@ def test_fetch_employee_record_list(self, api_client, mocker, faker): job_application = JobApplicationWithCompleteJobSeekerProfileFactory(to_company=self.siae) employee_record_rejected = EmployeeRecord.from_job_application(job_application=job_application) employee_record_rejected.ready() - employee_record_rejected.sent(faker.asp_batch_filename(), 1, None) + employee_record_rejected.sent(file=faker.asp_batch_filename(), line_number=1, archive=None) # There should be no result at this point response = api_client.get(ENDPOINT_URL + "?status=REJECTED", format="json") @@ -153,7 +153,7 @@ def test_fetch_employee_record_list(self, api_client, mocker, faker): assert len(result.get("results")) == 0 err_code, err_message = "12", "JSON Invalide" - employee_record_rejected.reject(err_code, err_message, None) + employee_record_rejected.reject(code=err_code, label=err_message, archive=None) # Status case is not important response = api_client.get(ENDPOINT_URL + "?status=rEjEcTeD", format="json") @@ -230,7 +230,7 @@ def test_status_array_parameter(self, api_client, mocker, faker): ) employee_record = EmployeeRecord.from_job_application(job_application_2) employee_record.ready() - employee_record.sent(faker.asp_batch_filename(), 1, None) + employee_record.sent(file=faker.asp_batch_filename(), line_number=1, archive=None) member = employee_record.job_application.to_company.members.first() api_client.force_login(member) diff --git a/tests/employee_record/test_models.py b/tests/employee_record/test_models.py index ad9dc60d4a..dce1b65478 100644 --- a/tests/employee_record/test_models.py +++ b/tests/employee_record/test_models.py @@ -348,29 +348,29 @@ def test_state_ready( assert self.employee_record.status == Status.READY def test_state_sent(self, faker): - self.employee_record.sent(faker.asp_batch_filename(), 42, "{}") + self.employee_record.sent(file=faker.asp_batch_filename(), line_number=42, archive="{}") assert self.employee_record.status == Status.SENT assert self.employee_record.asp_batch_line_number == 42 assert self.employee_record.archived_json == {} def test_state_rejected(self, faker): - self.employee_record.sent(faker.asp_batch_filename(), 1, None) + self.employee_record.sent(file=faker.asp_batch_filename(), line_number=1, archive=None) - self.employee_record.reject("12", "JSON Invalide", "{}") + self.employee_record.reject(code="12", label="JSON Invalide", archive="{}") assert self.employee_record.status == Status.REJECTED assert self.employee_record.asp_processing_code == "12" assert self.employee_record.asp_processing_label == "JSON Invalide" assert self.employee_record.archived_json == {} def test_state_processed(self, faker): - self.employee_record.sent(faker.asp_batch_filename(), 1, None) + self.employee_record.sent(file=faker.asp_batch_filename(), line_number=1, archive=None) process_code, process_message = ( EmployeeRecord.ASP_PROCESSING_SUCCESS_CODE, "La ligne de la fiche salarié a été enregistrée avec succès.", ) - self.employee_record.process(process_code, process_message, "{}") + self.employee_record.process(code=process_code, label=process_message, archive="{}") assert self.employee_record.status == Status.PROCESSED assert self.employee_record.asp_processing_code == process_code @@ -378,13 +378,13 @@ def test_state_processed(self, faker): assert self.employee_record.archived_json == {} def test_state_processed_when_archive_is_none(self, faker): - self.employee_record.sent(faker.asp_batch_filename(), 1, None) + self.employee_record.sent(file=faker.asp_batch_filename(), line_number=1, archive=None) process_code, process_message = ( EmployeeRecord.ASP_PROCESSING_SUCCESS_CODE, "La ligne de la fiche salarié a été enregistrée avec succès.", ) - self.employee_record.process(process_code, process_message, None) + self.employee_record.process(code=process_code, label=process_message, archive=None) assert self.employee_record.status == Status.PROCESSED assert self.employee_record.asp_processing_code == process_code @@ -392,13 +392,13 @@ def test_state_processed_when_archive_is_none(self, faker): assert self.employee_record.archived_json is None def test_state_processed_when_archive_is_empty(self, faker): - self.employee_record.sent(faker.asp_batch_filename(), 1, None) + self.employee_record.sent(file=faker.asp_batch_filename(), line_number=1, archive=None) process_code, process_message = ( EmployeeRecord.ASP_PROCESSING_SUCCESS_CODE, "La ligne de la fiche salarié a été enregistrée avec succès.", ) - self.employee_record.process(process_code, process_message, "") + self.employee_record.process(code=process_code, label=process_message, archive="") assert self.employee_record.status == Status.PROCESSED assert self.employee_record.asp_processing_code == process_code @@ -406,13 +406,13 @@ def test_state_processed_when_archive_is_empty(self, faker): assert self.employee_record.archived_json == "" def test_state_processed_when_archive_is_not_json(self, faker): - self.employee_record.sent(faker.asp_batch_filename(), 1, None) + self.employee_record.sent(file=faker.asp_batch_filename(), line_number=1, archive=None) process_code, process_message = ( EmployeeRecord.ASP_PROCESSING_SUCCESS_CODE, "La ligne de la fiche salarié a été enregistrée avec succès.", ) - self.employee_record.process(process_code, process_message, "whatever") + self.employee_record.process(code=process_code, label=process_message, archive="whatever") assert self.employee_record.status == Status.PROCESSED assert self.employee_record.asp_processing_code == process_code @@ -426,7 +426,7 @@ def test_state_disabled(self, faker): assert self.employee_record.status == Status.READY # Employee record in SENT state can't be disabled - self.employee_record.sent(faker.asp_batch_filename(), 1, None) + self.employee_record.sent(file=faker.asp_batch_filename(), line_number=1, archive=None) with pytest.raises(xworkflows.InvalidTransitionError): self.employee_record.disable() assert self.employee_record.status == Status.SENT @@ -436,7 +436,7 @@ def test_state_disabled(self, faker): EmployeeRecord.ASP_PROCESSING_SUCCESS_CODE, "La ligne de la fiche salarié a été enregistrée avec succès.", ) - self.employee_record.process(process_code, process_message, "{}") + self.employee_record.process(code=process_code, label=process_message, archive="{}") self.employee_record.disable() assert self.employee_record.status == Status.DISABLED @@ -451,18 +451,18 @@ def test_state_disabled(self, faker): assert self.employee_record.status == Status.DISABLED def test_state_disabled_with_reject(self, faker): - self.employee_record.sent(faker.asp_batch_filename(), 1, None) + self.employee_record.sent(file=faker.asp_batch_filename(), line_number=1, archive=None) - self.employee_record.reject("12", "JSON Invalide", None) + self.employee_record.reject(code="12", label="JSON Invalide", archive=None) self.employee_record.disable() assert self.employee_record.status == Status.DISABLED def test_reactivate(self, faker): - self.employee_record.sent(faker.unique.asp_batch_filename(), 1, None) + self.employee_record.sent(file=faker.unique.asp_batch_filename(), line_number=1, archive=None) process_code = EmployeeRecord.ASP_PROCESSING_SUCCESS_CODE process_message = "La ligne de la fiche salarié a été enregistrée avec succès." archive_first = '{"libelleTraitement":"La ligne de la fiche salarié a été enregistrée avec succès [1]."}' - self.employee_record.process(process_code, process_message, archive_first) + self.employee_record.process(code=process_code, label=process_message, archive=archive_first) self.employee_record.disable() assert self.employee_record.status == Status.DISABLED @@ -476,7 +476,7 @@ def test_reactivate(self, faker): filename_second = faker.unique.asp_batch_filename() archive_second = '{"libelleTraitement":"La ligne de la fiche salarié a été enregistrée avec succès [2]."}' - self.employee_record.sent(filename_second, 1, archive_second) + self.employee_record.sent(file=filename_second, line_number=1, archive=archive_second) assert self.employee_record.asp_batch_file == filename_second assert self.employee_record.archived_json == json.loads(archive_second) @@ -485,7 +485,7 @@ def test_reactivate(self, faker): "La ligne de la fiche salarié a été enregistrée avec succès.", ) archive_third = '{"libelleTraitement":"La ligne de la fiche salarié a été enregistrée avec succès [3]."}' - self.employee_record.process(process_code, process_message, archive_third) + self.employee_record.process(code=process_code, label=process_message, archive=archive_third) assert self.employee_record.asp_batch_file == filename_second assert self.employee_record.archived_json == json.loads(archive_third) @@ -496,8 +496,8 @@ def test_reactivate_when_the_siae_has_changed(self, faker): assert self.employee_record.siret == old_company.siret assert self.employee_record.asp_id == old_company.convention.asp_id - self.employee_record.sent(faker.unique.asp_batch_filename(), 1, None) - self.employee_record.process("", "", None) + self.employee_record.sent(file=faker.unique.asp_batch_filename(), line_number=1, archive=None) + self.employee_record.process(code="", label="", archive=None) self.employee_record.disable() # Change SIAE @@ -548,9 +548,9 @@ def test_processed_as_duplicate(self): asp_processing_label="Meh Meh Meh", ) employee_record_code_3436.process( - employee_record_code_3436.asp_processing_code, - employee_record_code_3436.asp_processing_label, - '{"codeTraitement": "3436"}', + code=employee_record_code_3436.asp_processing_code, + label=employee_record_code_3436.asp_processing_label, + archive='{"codeTraitement": "3436"}', as_duplicate=True, ) assert employee_record_code_3436.processed_as_duplicate @@ -560,17 +560,17 @@ def test_processed_as_duplicate(self): with pytest.raises(ValueError, match="Code needs to be 3436 and not 3437 when as_duplicate=True"): employee_record_other_code.process( - employee_record_other_code.asp_processing_code, - employee_record_other_code.asp_processing_label, - None, + code=employee_record_other_code.asp_processing_code, + label=employee_record_other_code.asp_processing_label, + archive=None, as_duplicate=True, ) with pytest.raises(xworkflows.InvalidTransitionError): employee_record_other_status.process( - employee_record_other_status.asp_processing_code, - employee_record_other_status.asp_processing_label, - None, + code=employee_record_other_status.asp_processing_code, + label=employee_record_other_status.asp_processing_label, + archive=None, as_duplicate=True, ) diff --git a/tests/www/employee_record_views/test_disable.py b/tests/www/employee_record_views/test_disable.py index 9453060138..beedd4adbb 100644 --- a/tests/www/employee_record_views/test_disable.py +++ b/tests/www/employee_record_views/test_disable.py @@ -55,7 +55,7 @@ def test_disable_employee_record_ready(self, client): def test_disable_employee_record_sent(self, client, faker): self.employee_record.ready() - self.employee_record.sent(faker.asp_batch_filename(), 1, None) + self.employee_record.sent(file=faker.asp_batch_filename(), line_number=1, archive=None) self.employee_record.refresh_from_db() assert self.employee_record.status == Status.SENT @@ -70,9 +70,9 @@ def test_disable_employee_record_sent(self, client, faker): def test_disable_employee_record_rejected(self, client, faker): self.employee_record.ready() - self.employee_record.sent(faker.asp_batch_filename(), 1, None) + self.employee_record.sent(file=faker.asp_batch_filename(), line_number=1, archive=None) err_code, err_message = "12", "JSON Invalide" - self.employee_record.reject(err_code, err_message, None) + self.employee_record.reject(code=err_code, label=err_message, archive=None) self.employee_record.refresh_from_db() assert self.employee_record.status == Status.REJECTED @@ -89,9 +89,9 @@ def test_disable_employee_record_rejected(self, client, faker): def test_disable_employee_record_completed(self, client, faker): self.employee_record.ready() - self.employee_record.sent(faker.asp_batch_filename(), 1, None) + self.employee_record.sent(file=faker.asp_batch_filename(), line_number=1, archive=None) process_code, process_message = "0000", "La ligne de la fiche salarié a été enregistrée avec succès." - self.employee_record.process(process_code, process_message, "{}") + self.employee_record.process(code=process_code, label=process_message, archive="{}") self.employee_record.refresh_from_db() assert self.employee_record.status == Status.PROCESSED diff --git a/tests/www/employee_record_views/test_list.py b/tests/www/employee_record_views/test_list.py index a714431fc9..94c39b14ab 100644 --- a/tests/www/employee_record_views/test_list.py +++ b/tests/www/employee_record_views/test_list.py @@ -252,8 +252,8 @@ def test_rejected_without_custom_message(self, client, faker): record = employee_record_factories.EmployeeRecordWithProfileFactory(job_application__to_company=self.company) record.ready() - record.sent(faker.asp_batch_filename(), 1, None) - record.reject("0012", "JSON Invalide", None) + record.sent(file=faker.asp_batch_filename(), line_number=1, archive=None) + record.reject(code="0012", label="JSON Invalide", archive=None) response = client.get(self.URL, data={"status": Status.REJECTED}) assertContains(response, "Erreur 0012") @@ -292,7 +292,7 @@ def test_rejected_custom_messages(self, client, subtests): for err_code, err_message, custom_err_message in tests_specs: with subtests.test(err_code): record.status = Status.SENT - record.reject(err_code, err_message, "{}") + record.reject(code=err_code, label=err_message, archive="{}") response = client.get(self.URL, data={"status": Status.REJECTED}) assertContains(response, f"Erreur {err_code}") @@ -367,8 +367,8 @@ def test_rejected_employee_records_sorted(self, client, snapshot): ) for i, record in enumerate((recordA, recordZ)): record.ready() - record.sent(f"RIAE_FS_2021041013000{i}.json", 1, None) - record.reject("0012", "JSON Invalide", None) + record.sent(file=f"RIAE_FS_2021041013000{i}.json", line_number=1, archive=None) + record.reject(code="0012", label="JSON Invalide", archive=None) # Zzzzz's hiring start is more recent self._check_employee_record_order( diff --git a/tests/www/employee_record_views/test_reactivate.py b/tests/www/employee_record_views/test_reactivate.py index 6f56aa026d..d8211c1b05 100644 --- a/tests/www/employee_record_views/test_reactivate.py +++ b/tests/www/employee_record_views/test_reactivate.py @@ -23,9 +23,9 @@ def setup_method(self): def test_reactivate_employee_record(self, client, faker): self.employee_record.ready() - self.employee_record.sent(faker.asp_batch_filename(), 1, None) + self.employee_record.sent(file=faker.asp_batch_filename(), line_number=1, archive=None) process_code, process_message = "0000", "La ligne de la fiche salarié a été enregistrée avec succès." - self.employee_record.process(process_code, process_message, "{}") + self.employee_record.process(code=process_code, label=process_message, archive="{}") self.employee_record.disable() client.force_login(self.user) diff --git a/tests/www/employee_record_views/test_summary.py b/tests/www/employee_record_views/test_summary.py index 3aeb754010..8aab2dfe50 100644 --- a/tests/www/employee_record_views/test_summary.py +++ b/tests/www/employee_record_views/test_summary.py @@ -40,7 +40,7 @@ def test_asp_batch_file_infos(self, client): assertNotContains(response, HORODATAGE) self.employee_record.ready() - self.employee_record.sent("RIAE_FS_20210410130000.json", 1, None) + self.employee_record.sent(file="RIAE_FS_20210410130000.json", line_number=1, archive=None) response = client.get(self.url) assertContains(response, HORODATAGE)