diff --git a/legal-api/src/legal_api/resources/v2/business/business.py b/legal-api/src/legal_api/resources/v2/business/business.py index 2936f6e64c..75c70ea008 100644 --- a/legal-api/src/legal_api/resources/v2/business/business.py +++ b/legal-api/src/legal_api/resources/v2/business/business.py @@ -90,8 +90,11 @@ def get_businesses(identifier: str): g.jwt_oidc_token_info, account_response) if orgs := account_response.get('orgs'): - if str(orgs[0].get('id')) == q_account: - business_json['accountId'] = orgs[0].get('id') + # A business can be affiliated in multiple accounts (in user account as well as in gov staff account's) + # AccountService.get_account_by_affiliated_identifier will fetch all of it + # check one of it has `q_account` + if any(str(org.get('id')) == q_account for org in orgs): + business_json['accountId'] = q_account return jsonify(business=business_json) diff --git a/legal-api/src/legal_api/services/__init__.py b/legal-api/src/legal_api/services/__init__.py index 7779b4b7ae..215003351b 100644 --- a/legal-api/src/legal_api/services/__init__.py +++ b/legal-api/src/legal_api/services/__init__.py @@ -20,16 +20,15 @@ from legal_api.models import Business from legal_api.utils.datetime import datetime -from .authz import ( +from .authz import ( # noqa: I001; ACCOUNT_IDENTITY, BASIC_USER, COLIN_SVC_ROLE, STAFF_ROLE, SYSTEM_ROLE, authorized, - get_account_by_affiliated_identifier, has_roles, -) +) # noqa: I001; from .bootstrap import AccountService, RegistrationBootstrapService from .business_details_version import VersionedBusinessDetailsService from .digital_credentials import DigitalCredentialsService diff --git a/legal-api/src/legal_api/services/authz.py b/legal-api/src/legal_api/services/authz.py index 7489b3da1c..3c3891c533 100644 --- a/legal-api/src/legal_api/services/authz.py +++ b/legal-api/src/legal_api/services/authz.py @@ -19,7 +19,6 @@ from urllib.parse import urljoin import jwt as pyjwt -import requests from flask import current_app from flask_jwt_oidc import JwtManager from requests import Session, exceptions @@ -753,23 +752,6 @@ def get_allowed(state: Business.State, legal_type: str, jwt: JwtManager): return allowable_filing_types -def get_account_by_affiliated_identifier(token, identifier: str): - """Return the account affiliated to the business.""" - auth_url = current_app.config.get('AUTH_SVC_URL') - url = f'{auth_url}/orgs?affiliation={identifier}' - - headers = { - 'Authorization': f'Bearer {token}' - } - - res = requests.get(url, headers=headers) - try: - return res.json() - except Exception: # noqa B902; pylint: disable=W0703; - current_app.logger.error('Failed to get response') - return None - - def add_allowable_filing_type(is_allowable: bool = False, allowable_filing_types: list = None, allowable_filing_type: dict = None): diff --git a/legal-api/src/legal_api/services/filings/validations/amalgamation_application.py b/legal-api/src/legal_api/services/filings/validations/amalgamation_application.py index 445d03c8de..fd5b68532f 100644 --- a/legal-api/src/legal_api/services/filings/validations/amalgamation_application.py +++ b/legal-api/src/legal_api/services/filings/validations/amalgamation_application.py @@ -116,9 +116,9 @@ def validate_amalgamating_businesses( # pylint: disable=too-many-branches,too-m 'error': f'Cannot amalgamate with {identifier} which is in historical state.', 'path': amalgamating_businesses_path }) - elif _has_future_effective_filing(amalgamating_business): + elif _has_pending_filing(amalgamating_business): msg.append({ - 'error': f'{identifier} has a future effective filing.', + 'error': f'{identifier} has a draft, pending or future effective filing.', 'path': amalgamating_businesses_path }) @@ -181,15 +181,18 @@ def validate_amalgamating_businesses( # pylint: disable=too-many-branches,too-m def _is_business_affliated(identifier, account_id): - if (account_response := AccountService.get_account_by_affiliated_identifier(identifier)) and \ - (orgs := account_response.get('orgs')) and str(orgs[0].get('id')) == account_id: + if ((account_response := AccountService.get_account_by_affiliated_identifier(identifier)) and + (orgs := account_response.get('orgs')) and + any(str(org.get('id')) == account_id for org in orgs)): return True return False -def _has_future_effective_filing(amalgamating_business: Business): - if Filing.get_filings_by_status(amalgamating_business.id, - [Filing.Status.PAID.value, Filing.Status.PENDING.value]): +def _has_pending_filing(amalgamating_business: Business): + if Filing.get_filings_by_status(amalgamating_business.id, [ + Filing.Status.DRAFT.value, + Filing.Status.PENDING.value, + Filing.Status.PAID.value]): return True return False diff --git a/legal-api/tests/unit/services/filings/validations/test_amalgamation_application.py b/legal-api/tests/unit/services/filings/validations/test_amalgamation_application.py index defa93e7e6..719a79ce13 100644 --- a/legal-api/tests/unit/services/filings/validations/test_amalgamation_application.py +++ b/legal-api/tests/unit/services/filings/validations/test_amalgamation_application.py @@ -663,7 +663,7 @@ def mock_find_by_identifier(identifier): mocker.patch('legal_api.services.filings.validations.amalgamation_application.validate_name_request', return_value=[]) - mocker.patch('legal_api.services.filings.validations.amalgamation_application._has_future_effective_filing', + mocker.patch('legal_api.services.filings.validations.amalgamation_application._has_pending_filing', return_value=False) mocker.patch('legal_api.models.business.Business.find_by_identifier', side_effect=mock_find_by_identifier) mocker.patch('legal_api.services.bootstrap.AccountService.get_account_by_affiliated_identifier', @@ -684,12 +684,12 @@ def mock_find_by_identifier(identifier): @pytest.mark.parametrize( 'test_status, expected_code, expected_msg', [ - ('FAIL', HTTPStatus.BAD_REQUEST, 'BC1234567 has a future effective filing.'), + ('FAIL', HTTPStatus.BAD_REQUEST, 'BC1234567 has a draft, pending or future effective filing.'), ('SUCCESS', None, None) ] ) -def test_has_future_effective_filing(mocker, app, session, jwt, test_status, expected_code, expected_msg): - """Assert valid amalgamating businesses has future effective filing.""" +def test_has_pending_filing(mocker, app, session, jwt, test_status, expected_code, expected_msg): + """Assert valid amalgamating businesses has draft, pending or future effective filing.""" filing = {'filing': {}} filing['filing']['header'] = {'name': 'amalgamationApplication', 'date': '2019-04-08', 'certifiedBy': 'full name', 'email': 'no_one@never.get', 'filingId': 1} @@ -746,7 +746,7 @@ def mock_find_by_identifier(identifier): mocker.patch('legal_api.services.filings.validations.amalgamation_application.validate_name_request', return_value=[]) - mocker.patch('legal_api.services.filings.validations.amalgamation_application._has_future_effective_filing', + mocker.patch('legal_api.services.filings.validations.amalgamation_application._has_pending_filing', return_value=False) mocker.patch('legal_api.models.business.Business.find_by_identifier', side_effect=mock_find_by_identifier) mocker.patch('legal_api.services.bootstrap.AccountService.get_account_by_affiliated_identifier', @@ -800,7 +800,7 @@ def mock_find_by_identifier(identifier): mocker.patch('legal_api.services.filings.validations.amalgamation_application.validate_name_request', return_value=[]) - mocker.patch('legal_api.services.filings.validations.amalgamation_application._has_future_effective_filing', + mocker.patch('legal_api.services.filings.validations.amalgamation_application._has_pending_filing', return_value=False) mocker.patch('legal_api.services.filings.validations.amalgamation_application._is_business_affliated', return_value=True) @@ -853,7 +853,7 @@ def mock_find_by_identifier(identifier): mocker.patch('legal_api.services.filings.validations.amalgamation_application.validate_name_request', return_value=[]) - mocker.patch('legal_api.services.filings.validations.amalgamation_application._has_future_effective_filing', + mocker.patch('legal_api.services.filings.validations.amalgamation_application._has_pending_filing', return_value=False) mocker.patch('legal_api.services.filings.validations.amalgamation_application._is_business_affliated', return_value=True) @@ -893,7 +893,7 @@ def mock_find_by_identifier(identifier): mocker.patch('legal_api.services.filings.validations.amalgamation_application.validate_name_request', return_value=[]) - mocker.patch('legal_api.services.filings.validations.amalgamation_application._has_future_effective_filing', + mocker.patch('legal_api.services.filings.validations.amalgamation_application._has_pending_filing', return_value=False) mocker.patch('legal_api.services.filings.validations.amalgamation_application._is_business_affliated', return_value=True) @@ -940,7 +940,7 @@ def mock_find_by_identifier(identifier): mocker.patch('legal_api.services.filings.validations.amalgamation_application.validate_name_request', return_value=[]) - mocker.patch('legal_api.services.filings.validations.amalgamation_application._has_future_effective_filing', + mocker.patch('legal_api.services.filings.validations.amalgamation_application._has_pending_filing', return_value=False) mocker.patch('legal_api.services.filings.validations.amalgamation_application._is_business_affliated', return_value=True) @@ -986,7 +986,7 @@ def mock_find_by_identifier(identifier): mocker.patch('legal_api.services.filings.validations.amalgamation_application.validate_name_request', return_value=[]) - mocker.patch('legal_api.services.filings.validations.amalgamation_application._has_future_effective_filing', + mocker.patch('legal_api.services.filings.validations.amalgamation_application._has_pending_filing', return_value=False) mocker.patch('legal_api.services.filings.validations.amalgamation_application._is_business_affliated', return_value=True) @@ -1033,7 +1033,7 @@ def mock_find_by_identifier(identifier): mocker.patch('legal_api.services.filings.validations.amalgamation_application.validate_name_request', return_value=[]) - mocker.patch('legal_api.services.filings.validations.amalgamation_application._has_future_effective_filing', + mocker.patch('legal_api.services.filings.validations.amalgamation_application._has_pending_filing', return_value=False) mocker.patch('legal_api.services.filings.validations.amalgamation_application._is_business_affliated', return_value=True) @@ -1090,7 +1090,7 @@ def mock_find_by_identifier(identifier): mocker.patch('legal_api.services.filings.validations.amalgamation_application.validate_name_request', return_value=[]) - mocker.patch('legal_api.services.filings.validations.amalgamation_application._has_future_effective_filing', + mocker.patch('legal_api.services.filings.validations.amalgamation_application._has_pending_filing', return_value=False) mocker.patch('legal_api.services.filings.validations.amalgamation_application._is_business_affliated', return_value=True)