mirror of
https://github.com/GSA/notifications-admin.git
synced 2026-08-23 15:57:23 -04:00
Fix unnecessary call to organisation API endpoint
We’re caching the organisation name, but still talking to the API to see if the organisation exists. `Service().organisation_id` only goes to the JSON for the service. `Service().organisation` makes a separate API call. We only need the former to know if a service belongs to an organisation.
This commit is contained in:
@@ -472,7 +472,7 @@ class Service(JSONModel):
|
|||||||
|
|
||||||
@property
|
@property
|
||||||
def organisation_name(self):
|
def organisation_name(self):
|
||||||
if not self.organisation:
|
if not self.organisation_id:
|
||||||
return None
|
return None
|
||||||
return organisations_client.get_organisation_name(self.organisation_id)
|
return organisations_client.get_organisation_name(self.organisation_id)
|
||||||
|
|
||||||
|
|||||||
@@ -296,7 +296,7 @@
|
|||||||
|
|
||||||
{% call row() %}
|
{% call row() %}
|
||||||
{{ text_field('Live')}}
|
{{ text_field('Live')}}
|
||||||
{% if current_service.trial_mode and not current_service.organisation %}
|
{% if current_service.trial_mode and not current_service.organisation_id %}
|
||||||
{{ text_field('No (organisation must be set first)') }}
|
{{ text_field('No (organisation must be set first)') }}
|
||||||
{{ text_field('') }}
|
{{ text_field('') }}
|
||||||
{% else %}
|
{% else %}
|
||||||
|
|||||||
@@ -1,5 +1,6 @@
|
|||||||
import uuid
|
import uuid
|
||||||
|
|
||||||
|
from app.models.organisation import Organisation
|
||||||
from app.models.service import Service
|
from app.models.service import Service
|
||||||
from app.models.user import User
|
from app.models.user import User
|
||||||
from tests import organisation_json
|
from tests import organisation_json
|
||||||
@@ -195,3 +196,54 @@ def test_organisation_type_when_service_and_its_org_both_have_an_org_type(mocker
|
|||||||
mocker.patch('app.organisations_client.get_organisation', return_value=org)
|
mocker.patch('app.organisations_client.get_organisation', return_value=org)
|
||||||
|
|
||||||
assert service.organisation_type == 'local'
|
assert service.organisation_type == 'local'
|
||||||
|
|
||||||
|
|
||||||
|
def test_organisation_name_comes_from_cache(mocker, service_one):
|
||||||
|
mock_redis_get = mocker.patch(
|
||||||
|
'app.extensions.RedisClient.get',
|
||||||
|
return_value=b'"Borchester Council"',
|
||||||
|
)
|
||||||
|
mock_get_organisation = mocker.patch('app.organisations_client.get_organisation')
|
||||||
|
service = Service(service_one)
|
||||||
|
service._dict['organisation'] = ORGANISATION_ID
|
||||||
|
|
||||||
|
assert service.organisation_name == 'Borchester Council'
|
||||||
|
mock_redis_get.assert_called_once_with(f'organisation-{ORGANISATION_ID}-name')
|
||||||
|
assert mock_get_organisation.called is False
|
||||||
|
|
||||||
|
|
||||||
|
def test_organisation_name_goes_into_cache(mocker, service_one):
|
||||||
|
mocker.patch(
|
||||||
|
'app.extensions.RedisClient.get',
|
||||||
|
return_value=None,
|
||||||
|
)
|
||||||
|
mock_redis_set = mocker.patch(
|
||||||
|
'app.extensions.RedisClient.set',
|
||||||
|
)
|
||||||
|
mocker.patch(
|
||||||
|
'app.organisations_client.get_organisation',
|
||||||
|
return_value=organisation_json(),
|
||||||
|
)
|
||||||
|
service = Service(service_one)
|
||||||
|
service._dict['organisation'] = ORGANISATION_ID
|
||||||
|
|
||||||
|
assert service.organisation_name == 'Test Organisation'
|
||||||
|
mock_redis_set.assert_called_once_with(
|
||||||
|
f'organisation-{ORGANISATION_ID}-name',
|
||||||
|
'"Test Organisation"',
|
||||||
|
ex=604800,
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def test_service_without_organisation_doesnt_need_org_api(mocker, service_one):
|
||||||
|
mock_redis_get = mocker.patch('app.extensions.RedisClient.get')
|
||||||
|
mock_get_organisation = mocker.patch('app.organisations_client.get_organisation')
|
||||||
|
service = Service(service_one)
|
||||||
|
service._dict['organisation'] = None
|
||||||
|
|
||||||
|
assert service.organisation_id is None
|
||||||
|
assert service.organisation_name is None
|
||||||
|
assert isinstance(service.organisation, Organisation)
|
||||||
|
|
||||||
|
assert mock_redis_get.called is False
|
||||||
|
assert mock_get_organisation.called is False
|
||||||
|
|||||||
Reference in New Issue
Block a user