From 5058f7239baf904dfb842042ea7fb71fd6573599 Mon Sep 17 00:00:00 2001 From: Akiff Manji Date: Thu, 30 Nov 2023 20:27:56 +0000 Subject: [PATCH 1/5] fix: add role to credential data with tests Signed-off-by: Akiff Manji --- .../legal_api/services/digital_credentials.py | 8 +- .../unit/services/test_digital_credentials.py | 100 +++++++++++++++++- 2 files changed, 104 insertions(+), 4 deletions(-) diff --git a/legal-api/src/legal_api/services/digital_credentials.py b/legal-api/src/legal_api/services/digital_credentials.py index 5ab516782f..cb7cd3e933 100644 --- a/legal-api/src/legal_api/services/digital_credentials.py +++ b/legal-api/src/legal_api/services/digital_credentials.py @@ -292,6 +292,12 @@ def get_digital_credential_data(business: Business, user: User, credential_type: given_names = ' '.join([x.strip() for x in [user.firstname, user.middlename] if x and x.strip()]).upper() + # For an SP there is only one role. This will need to be updated + # when the entity model changes and we need to support multiple roles. + role = ( + business.party_roles[0].role if (business.party_roles and len(business.party_roles.all())) else '' + ).replace('_', ' ').title() + return [ { 'name': 'credential_id', @@ -331,7 +337,7 @@ def get_digital_credential_data(business: Business, user: User, credential_type: }, { 'name': 'role', - 'value': '' + 'value': role or '' } ] diff --git a/legal-api/tests/unit/services/test_digital_credentials.py b/legal-api/tests/unit/services/test_digital_credentials.py index c938a2f9fb..c46266636a 100644 --- a/legal-api/tests/unit/services/test_digital_credentials.py +++ b/legal-api/tests/unit/services/test_digital_credentials.py @@ -17,14 +17,16 @@ """ from unittest.mock import MagicMock -from legal_api.models import DCDefinition +import pytest +from legal_api.models import DCDefinition, DCIssuedBusinessUserCredential, PartyRole from legal_api.services import digital_credentials -from legal_api.services.digital_credentials import DigitalCredentialsService +from legal_api.services.digital_credentials import DigitalCredentialsHelpers, DigitalCredentialsService +from tests.unit.models import factory_business, factory_user schema_id = 'test_schema_id' cred_def_id = 'test_credential_definition_id' -def test_init_app(session, app): # pylint:disable=unused-argument +def test_init_app(app, session): """Assert that the init app register schema and credential definition.""" DigitalCredentialsService._fetch_schema = MagicMock(return_value=schema_id) DigitalCredentialsService._fetch_credential_definition = MagicMock(return_value=cred_def_id) @@ -36,3 +38,95 @@ def test_init_app(session, app): # pylint:disable=unused-argument assert definition.schema_version == digital_credentials.business_schema_version assert definition.credential_definition_id == cred_def_id assert not definition.is_deleted + + +@pytest.mark.parametrize('test_data', [{ + 'business': { + 'identifier': 'FM1234567', + 'entity_type': 'SP', + 'founding_date': '2010-01-01', + 'state': 'ACTIVE', + }, + 'business_extra': { + 'legal_name': 'Test Business', + 'tax_id': '000000000000001', + }, + 'party_roles': [{ + 'role': 'proprietor' + }], + 'user': { + 'username': 'test', + 'lastname': 'Last', + 'firstname': 'First', + }, + 'user_extra': { + 'middlename': 'Middle', + }, + 'expected': [ + {'name': 'credential_id', 'value': '00000001'}, + {'name': 'identifier', 'value': 'FM1234567'}, + {'name': 'business_name', 'value': 'Test Business'}, + {'name': 'business_type', 'value': 'BC Sole Proprietorship'}, + {'name': 'cra_business_number', 'value': '000000000000001'}, + {'name': 'registered_on_dateint', 'value': '20100101'}, + {'name': 'company_status', 'value': 'ACTIVE'}, + {'name': 'family_name', 'value': 'LAST'}, + {'name': 'given_names', 'value': 'FIRST MIDDLE'}, + {'name': 'role', 'value': 'Proprietor'} + ] +}, { + 'business': { + 'identifier': 'FM1234567' + }, + 'business_extra': { + 'legal_name': '', + 'tax_id': '', + }, + 'party_roles': [{ + 'role': '' + }], + 'user': { + 'username': 'test' + }, + 'user_extra': { + 'middlename': '', + }, + 'expected': [ + {'name': 'credential_id', 'value': '00000002'}, + {'name': 'identifier', 'value': 'FM1234567'}, + {'name': 'business_name', 'value': ''}, + {'name': 'business_type', 'value': 'BC Cooperative Association'}, + {'name': 'cra_business_number', 'value': ''}, + {'name': 'registered_on_dateint', 'value': '19700101'}, + {'name': 'company_status', 'value': 'ACTIVE'}, + {'name': 'family_name', 'value': ''}, + {'name': 'given_names', 'value': ''}, + {'name': 'role', 'value': ''} + ] +}]) +def test_data_helper(app, session, test_data): + """Assert that the data helper returns the correct data.""" + # Arrange + credential_type = DCDefinition.CredentialType.business + + user = factory_user(**test_data['user']) + user.middlename = test_data['user_extra']['middlename'] + user.save() + + business = factory_business(**test_data['business']) + business.legal_name = test_data['business_extra']['legal_name'] + business.tax_id = test_data['business_extra']['tax_id'] + business.save() + + for party_role in test_data['party_roles']: + _party_role = PartyRole(**party_role) + _party_role.business_id = business.id + _party_role.save() + + issued_business_user_credential = DCIssuedBusinessUserCredential(business_id=business.id, user_id=user.id) + + # Act + credential_data = DigitalCredentialsHelpers.get_digital_credential_data(business, user, credential_type) + + # Assert + assert credential_data == test_data['expected'] \ No newline at end of file From 24cfb0752159e7c1385e68e9d54c8fc4423e9aed Mon Sep 17 00:00:00 2001 From: Akiff Manji Date: Wed, 29 Nov 2023 13:07:05 -0800 Subject: [PATCH 2/5] feat: digital credentials decorator (#2337) * feat: adds check for digital business card feature Signed-off-by: Akiff Manji * refactor: better function naming Signed-off-by: Akiff Manji * feat: decorator pto protect digital credentials endpoints Signed-off-by: Akiff Manji * chore: address code review comments Signed-off-by: Akiff Manji --------- Signed-off-by: Akiff Manji --- legal-api/tests/unit/services/test_digital_credentials.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/legal-api/tests/unit/services/test_digital_credentials.py b/legal-api/tests/unit/services/test_digital_credentials.py index c46266636a..5ab65ce105 100644 --- a/legal-api/tests/unit/services/test_digital_credentials.py +++ b/legal-api/tests/unit/services/test_digital_credentials.py @@ -129,4 +129,4 @@ def test_data_helper(app, session, test_data): credential_data = DigitalCredentialsHelpers.get_digital_credential_data(business, user, credential_type) # Assert - assert credential_data == test_data['expected'] \ No newline at end of file + assert credential_data == test_data['expected'] From a1df168f9de54349ca26dd9712c8730cffb132e8 Mon Sep 17 00:00:00 2001 From: Akiff Manji Date: Fri, 1 Dec 2023 22:08:36 +0000 Subject: [PATCH 3/5] fix: test failures Signed-off-by: Akiff Manji --- .../tests/unit/services/test_digital_credentials.py | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/legal-api/tests/unit/services/test_digital_credentials.py b/legal-api/tests/unit/services/test_digital_credentials.py index 5ab65ce105..82fd74eb71 100644 --- a/legal-api/tests/unit/services/test_digital_credentials.py +++ b/legal-api/tests/unit/services/test_digital_credentials.py @@ -63,7 +63,7 @@ def test_init_app(app, session): 'middlename': 'Middle', }, 'expected': [ - {'name': 'credential_id', 'value': '00000001'}, + {'name': 'credential_id', 'value': ''}, {'name': 'identifier', 'value': 'FM1234567'}, {'name': 'business_name', 'value': 'Test Business'}, {'name': 'business_type', 'value': 'BC Sole Proprietorship'}, @@ -92,7 +92,7 @@ def test_init_app(app, session): 'middlename': '', }, 'expected': [ - {'name': 'credential_id', 'value': '00000002'}, + {'name': 'credential_id', 'value': ''}, {'name': 'identifier', 'value': 'FM1234567'}, {'name': 'business_name', 'value': ''}, {'name': 'business_type', 'value': 'BC Cooperative Association'}, @@ -124,9 +124,14 @@ def test_data_helper(app, session, test_data): _party_role.save() issued_business_user_credential = DCIssuedBusinessUserCredential(business_id=business.id, user_id=user.id) + issued_business_user_credential.save() # Act credential_data = DigitalCredentialsHelpers.get_digital_credential_data(business, user, credential_type) # Assert - assert credential_data == test_data['expected'] + for item in credential_data: + if item['name'] == 'credential_id': + assert item['value'] == f'{issued_business_user_credential.id:08}' + else: + assert item in test_data['expected'] \ No newline at end of file From 16973a221b69e724147942d6b32255482a65531d Mon Sep 17 00:00:00 2001 From: Akiff Manji Date: Fri, 1 Dec 2023 22:55:51 +0000 Subject: [PATCH 4/5] refactor: update message for admin revocation Signed-off-by: Akiff Manji --- legal-api/src/legal_api/models/dc_revocation_reason.py | 1 + .../digital_credentials_processors/admin_revoke.py | 2 +- .../unit/digital_credentials_processors/test_admin_revoke.py | 2 +- 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/legal-api/src/legal_api/models/dc_revocation_reason.py b/legal-api/src/legal_api/models/dc_revocation_reason.py index 150e552c53..0dc8d1e24d 100644 --- a/legal-api/src/legal_api/models/dc_revocation_reason.py +++ b/legal-api/src/legal_api/models/dc_revocation_reason.py @@ -18,6 +18,7 @@ class DCRevocationReason(Enum): """Digital Credential Revocation Reasons.""" + ADMINISTRATIVE_REVOCATION = 'Your credential was revoked.' UPDATED_INFORMATION = 'You were offered a new credential with updated information ' \ 'and that revoked all previous copies.' VOLUNTARY_DISSOLUTION = 'You chose to dissolve your business. ' \ diff --git a/queue_services/entity-digital-credentials/src/entity_digital_credentials/digital_credentials_processors/admin_revoke.py b/queue_services/entity-digital-credentials/src/entity_digital_credentials/digital_credentials_processors/admin_revoke.py index f381d92854..be9e493e8c 100644 --- a/queue_services/entity-digital-credentials/src/entity_digital_credentials/digital_credentials_processors/admin_revoke.py +++ b/queue_services/entity-digital-credentials/src/entity_digital_credentials/digital_credentials_processors/admin_revoke.py @@ -29,4 +29,4 @@ async def process(business: Business): return revoke_issued_digital_credential(business=business, issued_credential=issued_credentials[0], - reason=DCRevocationReason.UPDATED_INFORMATION) + reason=DCRevocationReason.ADMINISTRATIVE_REVOCATION) diff --git a/queue_services/entity-digital-credentials/tests/unit/digital_credentials_processors/test_admin_revoke.py b/queue_services/entity-digital-credentials/tests/unit/digital_credentials_processors/test_admin_revoke.py index 517e3d2479..bc18b30396 100644 --- a/queue_services/entity-digital-credentials/tests/unit/digital_credentials_processors/test_admin_revoke.py +++ b/queue_services/entity-digital-credentials/tests/unit/digital_credentials_processors/test_admin_revoke.py @@ -62,4 +62,4 @@ async def test_processor_revokes_issued_credential(mock_revoke_issued_digital_cr mock_get_issued_digital_credentials.assert_called_once_with(business=business) mock_revoke_issued_digital_credential.assert_called_once_with(business=business, issued_credential={'id': 1}, - reason=DCRevocationReason.UPDATED_INFORMATION) + reason=DCRevocationReason.ADMINISTRATIVE_REVOCATION) From 77cf97ad58a28a1ad5195babd4b2a36727e56027 Mon Sep 17 00:00:00 2001 From: Akiff Manji Date: Mon, 4 Dec 2023 17:51:17 +0000 Subject: [PATCH 5/5] chore: temporarily remove admin revoke updates --- .../digital_credentials_processors/admin_revoke.py | 2 +- .../unit/digital_credentials_processors/test_admin_revoke.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/queue_services/entity-digital-credentials/src/entity_digital_credentials/digital_credentials_processors/admin_revoke.py b/queue_services/entity-digital-credentials/src/entity_digital_credentials/digital_credentials_processors/admin_revoke.py index be9e493e8c..f381d92854 100644 --- a/queue_services/entity-digital-credentials/src/entity_digital_credentials/digital_credentials_processors/admin_revoke.py +++ b/queue_services/entity-digital-credentials/src/entity_digital_credentials/digital_credentials_processors/admin_revoke.py @@ -29,4 +29,4 @@ async def process(business: Business): return revoke_issued_digital_credential(business=business, issued_credential=issued_credentials[0], - reason=DCRevocationReason.ADMINISTRATIVE_REVOCATION) + reason=DCRevocationReason.UPDATED_INFORMATION) diff --git a/queue_services/entity-digital-credentials/tests/unit/digital_credentials_processors/test_admin_revoke.py b/queue_services/entity-digital-credentials/tests/unit/digital_credentials_processors/test_admin_revoke.py index bc18b30396..517e3d2479 100644 --- a/queue_services/entity-digital-credentials/tests/unit/digital_credentials_processors/test_admin_revoke.py +++ b/queue_services/entity-digital-credentials/tests/unit/digital_credentials_processors/test_admin_revoke.py @@ -62,4 +62,4 @@ async def test_processor_revokes_issued_credential(mock_revoke_issued_digital_cr mock_get_issued_digital_credentials.assert_called_once_with(business=business) mock_revoke_issued_digital_credential.assert_called_once_with(business=business, issued_credential={'id': 1}, - reason=DCRevocationReason.ADMINISTRATIVE_REVOCATION) + reason=DCRevocationReason.UPDATED_INFORMATION)