diff --git a/app/__init__.py b/app/__init__.py index 792142baf..f39d4eaae 100644 --- a/app/__init__.py +++ b/app/__init__.py @@ -41,7 +41,8 @@ from werkzeug.local import LocalProxy from app import proxy_fix from app.config import configs from app.asset_fingerprinter import asset_fingerprinter -from app.notify_client.models import Service +from app.models.service import Service +from app.models.user import AnonymousUser from app.navigation import ( CaseworkNavigation, HeaderNavigation, @@ -59,7 +60,6 @@ from app.notify_client.user_api_client import user_api_client from app.notify_client.events_api_client import events_api_client from app.notify_client.provider_client import provider_client from app.notify_client.email_branding_client import email_branding_client -from app.notify_client.models import AnonymousUser from app.notify_client.organisations_api_client import organisations_client from app.notify_client.org_invite_api_client import org_invite_api_client from app.notify_client.letter_jobs_client import letter_jobs_client diff --git a/app/main/forms.py b/app/main/forms.py index 91df7bc59..643fdcd31 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -43,7 +43,7 @@ from app.main.validators import ( ValidEmail, ValidGovEmail, ) -from app.notify_client.models import permissions, roles +from app.models.user import permissions, roles from app.utils import AgreementInfo, guess_name_from_email_address diff --git a/app/main/views/add_service.py b/app/main/views/add_service.py index e311a70e6..114f43437 100644 --- a/app/main/views/add_service.py +++ b/app/main/views/add_service.py @@ -12,7 +12,7 @@ from app import ( ) from app.main import main from app.main.forms import CreateServiceForm -from app.notify_client.models import InvitedUser +from app.models.user import InvitedUser from app.utils import AgreementInfo, email_safe, is_gov_user diff --git a/app/main/views/manage_users.py b/app/main/views/manage_users.py index 90b98450e..4c0642ead 100644 --- a/app/main/views/manage_users.py +++ b/app/main/views/manage_users.py @@ -10,7 +10,7 @@ from app import ( ) from app.main import main from app.main.forms import InviteUserForm, PermissionsForm, SearchUsersForm -from app.notify_client.models import permissions +from app.models.user import permissions from app.utils import user_has_permissions diff --git a/app/models/service.py b/app/models/service.py new file mode 100644 index 000000000..9d59f2e9a --- /dev/null +++ b/app/models/service.py @@ -0,0 +1,157 @@ +from werkzeug.utils import cached_property + +from app.utils import get_default_sms_sender + + +class Service(): + + ALLOWED_PROPERTIES = { + 'active', + 'contact_link', + 'dvla_organisation', + 'email_branding', + 'email_from', + 'id', + 'inbound_api', + 'letter_contact_block', + 'letter_logo_filename', + 'message_limit', + 'name', + 'organisation_type', + 'permissions', + 'postage', + 'prefix_sms', + 'research_mode', + 'service_callback_api', + } + + def __init__(self, _dict): + # in the case of a bad request current service may be `None` + self._dict = _dict or {} + if 'permissions' not in self._dict: + self.permissions = {'email', 'sms', 'letter'} + + def __bool__(self): + return self._dict != {} + + def __getattr__(self, attr): + if attr in self.ALLOWED_PROPERTIES: + return self._dict[attr] + raise AttributeError('`{}` is not a service attribute'.format(attr)) + + def __getitem__(self, attr): + raise NotImplementedError( + 'Use current_service.{} instead of current_service[\'{}\']'.format(attr, attr) + ) + + def get(self, attr, default=None): + try: + return self._dict[attr] + except KeyError: + return default + + @property + def trial_mode(self): + return self._dict['restricted'] + + def has_permission(self, permission): + return permission in self.permissions + + @cached_property + def has_jobs(self): + # Can’t import at top-level because app isn’t yet initialised + from app import job_api_client + return job_api_client.has_jobs(self.id) + + @cached_property + def has_team_members(self): + from app import user_api_client + return user_api_client.get_count_of_users_with_permission( + self.id, 'manage_service' + ) > 1 + + @cached_property + def templates(self): + + from app import service_api_client + + templates = service_api_client.get_service_templates(self.id)['data'] + + return [ + template for template in templates + if template['template_type'] in self.available_template_types + ] + + def templates_by_type(self, template_type): + return [ + template for template in self.templates + if template_type in {'all', template['template_type']} + ] + + @property + def available_template_types(self): + return [ + channel for channel in ('email', 'sms', 'letter') + if self.has_permission(channel) + ] + + @property + def has_templates(self): + return len(self.templates) > 0 + + @property + def has_multiple_template_types(self): + return len({ + template['template_type'] for template in self.templates + }) > 1 + + @property + def has_email_templates(self): + return len(self.templates_by_type('email')) > 0 + + @property + def has_sms_templates(self): + return len(self.templates_by_type('sms')) > 0 + + @cached_property + def has_email_reply_to_address(self): + from app import service_api_client + return bool(service_api_client.get_reply_to_email_addresses( + self.id + )) + + @property + def needs_to_add_email_reply_to_address(self): + return self.has_email_templates and not self.has_email_reply_to_address + + @property + def shouldnt_use_govuk_as_sms_sender(self): + return self.organisation_type in {'local', 'nhs'} + + @cached_property + def sms_sender_is_govuk(self): + from app import service_api_client + return get_default_sms_sender( + service_api_client.get_sms_senders(self.id) + ) in {'GOVUK', 'None'} + + @property + def needs_to_change_sms_sender(self): + return all(( + self.has_sms_templates, + self.shouldnt_use_govuk_as_sms_sender, + self.sms_sender_is_govuk, + )) + + @property + def go_live_checklist_completed(self): + return all(( + self.has_team_members, + self.has_templates, + not self.needs_to_add_email_reply_to_address, + not self.needs_to_change_sms_sender, + )) + + @property + def go_live_checklist_completed_as_yes_no(self): + return 'Yes' if self.go_live_checklist_completed else 'No' diff --git a/app/notify_client/models.py b/app/models/user.py similarity index 68% rename from app/notify_client/models.py rename to app/models/user.py index cc520e124..201743356 100644 --- a/app/notify_client/models.py +++ b/app/models/user.py @@ -2,9 +2,6 @@ from itertools import chain from flask import request, session from flask_login import AnonymousUserMixin, UserMixin -from werkzeug.utils import cached_property - -from app.utils import get_default_sms_sender roles = { 'send_messages': ['send_texts', 'send_emails', 'send_letters'], @@ -267,157 +264,3 @@ class AnonymousUser(AnonymousUserMixin): # set the anonymous user so that if a new browser hits us we don't error http://stackoverflow.com/a/19275188 def logged_in_elsewhere(self): return False - - -class Service(): - - ALLOWED_PROPERTIES = { - 'active', - 'contact_link', - 'dvla_organisation', - 'email_branding', - 'email_from', - 'id', - 'inbound_api', - 'letter_contact_block', - 'letter_logo_filename', - 'message_limit', - 'name', - 'organisation_type', - 'permissions', - 'postage', - 'prefix_sms', - 'research_mode', - 'service_callback_api', - } - - def __init__(self, _dict): - # in the case of a bad request current service may be `None` - self._dict = _dict or {} - if 'permissions' not in self._dict: - self.permissions = {'email', 'sms', 'letter'} - - def __bool__(self): - return self._dict != {} - - def __getattr__(self, attr): - if attr in self.ALLOWED_PROPERTIES: - return self._dict[attr] - raise AttributeError('`{}` is not a service attribute'.format(attr)) - - def __getitem__(self, attr): - raise NotImplementedError( - 'Use current_service.{} instead of current_service[\'{}\']'.format(attr, attr) - ) - - def get(self, attr, default=None): - try: - return self._dict[attr] - except KeyError: - return default - - @property - def trial_mode(self): - return self._dict['restricted'] - - def has_permission(self, permission): - return permission in self.permissions - - @cached_property - def has_jobs(self): - # Can’t import at top-level because app isn’t yet initialised - from app import job_api_client - return job_api_client.has_jobs(self.id) - - @cached_property - def has_team_members(self): - from app import user_api_client - return user_api_client.get_count_of_users_with_permission( - self.id, 'manage_service' - ) > 1 - - @cached_property - def templates(self): - - from app import service_api_client - - templates = service_api_client.get_service_templates(self.id)['data'] - - return [ - template for template in templates - if template['template_type'] in self.available_template_types - ] - - def templates_by_type(self, template_type): - return [ - template for template in self.templates - if template_type in {'all', template['template_type']} - ] - - @property - def available_template_types(self): - return [ - channel for channel in ('email', 'sms', 'letter') - if self.has_permission(channel) - ] - - @property - def has_templates(self): - return len(self.templates) > 0 - - @property - def has_multiple_template_types(self): - return len({ - template['template_type'] for template in self.templates - }) > 1 - - @property - def has_email_templates(self): - return len(self.templates_by_type('email')) > 0 - - @property - def has_sms_templates(self): - return len(self.templates_by_type('sms')) > 0 - - @cached_property - def has_email_reply_to_address(self): - from app import service_api_client - return bool(service_api_client.get_reply_to_email_addresses( - self.id - )) - - @property - def needs_to_add_email_reply_to_address(self): - return self.has_email_templates and not self.has_email_reply_to_address - - @property - def shouldnt_use_govuk_as_sms_sender(self): - return self.organisation_type in {'local', 'nhs'} - - @cached_property - def sms_sender_is_govuk(self): - from app import service_api_client - return get_default_sms_sender( - service_api_client.get_sms_senders(self.id) - ) in {'GOVUK', 'None'} - - @property - def needs_to_change_sms_sender(self): - return all(( - self.has_sms_templates, - self.shouldnt_use_govuk_as_sms_sender, - self.sms_sender_is_govuk, - )) - - @property - def go_live_checklist_completed(self): - return all(( - self.has_team_members, - self.has_templates, - not self.needs_to_add_email_reply_to_address, - not self.needs_to_change_sms_sender, - )) - - @property - def go_live_checklist_completed_as_yes_no(self): - return 'Yes' if self.go_live_checklist_completed else 'No' diff --git a/app/notify_client/invite_api_client.py b/app/notify_client/invite_api_client.py index f9a9c0b6c..9ea677157 100644 --- a/app/notify_client/invite_api_client.py +++ b/app/notify_client/invite_api_client.py @@ -1,8 +1,8 @@ -from app.notify_client import NotifyAdminAPIClient, _attach_current_user, cache -from app.notify_client.models import ( +from app.models.user import ( InvitedUser, translate_permissions_from_admin_roles_to_db, ) +from app.notify_client import NotifyAdminAPIClient, _attach_current_user, cache class InviteApiClient(NotifyAdminAPIClient): diff --git a/app/notify_client/org_invite_api_client.py b/app/notify_client/org_invite_api_client.py index c3fa632c1..f80b20ca3 100644 --- a/app/notify_client/org_invite_api_client.py +++ b/app/notify_client/org_invite_api_client.py @@ -1,5 +1,5 @@ +from app.models.user import InvitedOrgUser from app.notify_client import NotifyAdminAPIClient, _attach_current_user -from app.notify_client.models import InvitedOrgUser class OrgInviteApiClient(NotifyAdminAPIClient): diff --git a/app/notify_client/user_api_client.py b/app/notify_client/user_api_client.py index 6f1daf6b8..78a694c88 100644 --- a/app/notify_client/user_api_client.py +++ b/app/notify_client/user_api_client.py @@ -2,12 +2,12 @@ from itertools import chain from notifications_python_client.errors import HTTPError -from app.notify_client import NotifyAdminAPIClient, cache -from app.notify_client.models import ( +from app.models.user import ( User, roles, translate_permissions_from_admin_roles_to_db, ) +from app.notify_client import NotifyAdminAPIClient, cache ALLOWED_ATTRIBUTES = { 'name', diff --git a/tests/__init__.py b/tests/__init__.py index 6a50ffcd6..f12e111b8 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -6,9 +6,7 @@ from datetime import datetime, timedelta, timezone from flask.testing import FlaskClient from flask import url_for from flask_login import login_user -from app.notify_client.models import ( - InvitedOrgUser, -) +from app.models.user import InvitedOrgUser class TestClient(FlaskClient): diff --git a/tests/app/main/test_permissions.py b/tests/app/main/test_permissions.py index 08ac18bd5..70b013df7 100644 --- a/tests/app/main/test_permissions.py +++ b/tests/app/main/test_permissions.py @@ -3,7 +3,7 @@ from flask import request from werkzeug.exceptions import Forbidden, Unauthorized from app.main.views.index import index -from app.notify_client.models import ( +from app.models.user import ( translate_permissions_from_admin_roles_to_db, translate_permissions_from_db_to_admin_roles, ) diff --git a/tests/app/main/views/accounts/test_show_accounts_or_dashboard.py b/tests/app/main/views/accounts/test_show_accounts_or_dashboard.py index 6a0582be7..5eb5117b6 100644 --- a/tests/app/main/views/accounts/test_show_accounts_or_dashboard.py +++ b/tests/app/main/views/accounts/test_show_accounts_or_dashboard.py @@ -1,7 +1,7 @@ import pytest from flask import url_for -from app.notify_client.models import User +from app.models.user import User from tests import user_json diff --git a/tests/app/main/views/organisations/test_organisation_invites.py b/tests/app/main/views/organisations/test_organisation_invites.py index 10f5a7647..8960acbf2 100644 --- a/tests/app/main/views/organisations/test_organisation_invites.py +++ b/tests/app/main/views/organisations/test_organisation_invites.py @@ -6,7 +6,7 @@ from bs4 import BeautifulSoup from flask import url_for from notifications_python_client.errors import HTTPError -from app.notify_client.models import InvitedOrgUser +from app.models.user import InvitedOrgUser from tests.conftest import ORGANISATION_ID, normalize_spaces diff --git a/tests/app/main/views/test_accept_invite.py b/tests/app/main/views/test_accept_invite.py index 56c18dcfb..6f22eabdb 100644 --- a/tests/app/main/views/test_accept_invite.py +++ b/tests/app/main/views/test_accept_invite.py @@ -6,7 +6,7 @@ from flask import url_for from notifications_python_client.errors import HTTPError import app -from app.notify_client.models import InvitedUser +from app.models.user import InvitedUser from tests.conftest import ( SERVICE_ONE_ID, active_caseworking_user, diff --git a/tests/app/main/views/test_activity.py b/tests/app/main/views/test_activity.py index a97744d66..01819a4d4 100644 --- a/tests/app/main/views/test_activity.py +++ b/tests/app/main/views/test_activity.py @@ -9,7 +9,7 @@ from flask import url_for from freezegun import freeze_time from app.main.views.jobs import get_status_filters, get_time_left -from app.notify_client.models import Service +from app.models.service import Service from tests.conftest import ( SERVICE_ONE_ID, active_caseworking_user, diff --git a/tests/app/main/views/test_manage_users.py b/tests/app/main/views/test_manage_users.py index 71cc6b836..e3a7b7590 100644 --- a/tests/app/main/views/test_manage_users.py +++ b/tests/app/main/views/test_manage_users.py @@ -5,7 +5,7 @@ from bs4 import BeautifulSoup from flask import url_for import app -from app.notify_client.models import InvitedUser +from app.models.user import InvitedUser from app.utils import is_gov_user from tests.conftest import ( SERVICE_ONE_ID, diff --git a/tests/app/main/views/test_register.py b/tests/app/main/views/test_register.py index d5cd1847c..5776afc98 100644 --- a/tests/app/main/views/test_register.py +++ b/tests/app/main/views/test_register.py @@ -6,7 +6,7 @@ from bs4 import BeautifulSoup from flask import session, url_for from flask_login import current_user -from app.notify_client.models import InvitedUser +from app.models.user import InvitedUser def test_render_register_returns_template_with_form(client): diff --git a/tests/app/main/views/test_service_settings.py b/tests/app/main/views/test_service_settings.py index 85b4c69dd..52d4af555 100644 --- a/tests/app/main/views/test_service_settings.py +++ b/tests/app/main/views/test_service_settings.py @@ -563,13 +563,13 @@ def test_should_show_request_to_go_live_checklist( ) mock_templates = mocker.patch( - 'app.notify_client.models.Service.templates', + 'app.models.service.Service.templates', new_callable=PropertyMock, return_value=list(range(0, count_of_templates)), ) mock_templates_by_type = mocker.patch( - 'app.notify_client.models.Service.templates_by_type', + 'app.models.service.Service.templates_by_type', side_effect=_templates_by_type, ) @@ -683,12 +683,12 @@ def test_should_check_for_sms_sender_on_go_live( return_value=99, ) mock_templates = mocker.patch( - 'app.notify_client.models.Service.templates', + 'app.models.service.Service.templates', new_callable=PropertyMock, side_effect=partial(_templates_by_type, 'all'), ) mock_templates_by_type = mocker.patch( - 'app.notify_client.models.Service.templates_by_type', + 'app.models.service.Service.templates_by_type', side_effect=_templates_by_type, ) @@ -747,7 +747,7 @@ def test_should_check_for_mou_on_request_to_go_live( return_value=0, ) mocker.patch( - 'app.notify_client.models.Service.templates', + 'app.models.service.Service.templates', new_callable=PropertyMock, return_value=[], ) @@ -1022,16 +1022,16 @@ def test_ready_to_go_live( 'sms_sender_is_govuk', }: mocker.patch( - 'app.notify_client.models.Service.{}'.format(prop), + 'app.models.service.Service.{}'.format(prop), new_callable=PropertyMock ).return_value = locals()[prop] - assert app.notify_client.models.Service({ + assert app.models.service.Service({ 'id': SERVICE_ONE_ID }).go_live_checklist_completed_as_yes_no == expected_readyness assert list(app.main.views.service_settings._get_request_to_go_live_tags( - app.notify_client.models.Service({'id': SERVICE_ONE_ID}), + app.models.service.Service({'id': SERVICE_ONE_ID}), agreement_signed, )) == expected_tags diff --git a/tests/app/notify_client/test_notify_admin_api_client.py b/tests/app/notify_client/test_notify_admin_api_client.py index 3f127abad..2e542f87e 100644 --- a/tests/app/notify_client/test_notify_admin_api_client.py +++ b/tests/app/notify_client/test_notify_admin_api_client.py @@ -4,8 +4,8 @@ from unittest.mock import patch import pytest import werkzeug +from app.models.service import Service from app.notify_client import NotifyAdminAPIClient -from app.notify_client.models import Service from tests import service_json from tests.conftest import api_user_active, platform_admin_user, set_config diff --git a/tests/app/notify_client/test_user_client.py b/tests/app/notify_client/test_user_client.py index 33d0b1802..6eebb9cd2 100644 --- a/tests/app/notify_client/test_user_client.py +++ b/tests/app/notify_client/test_user_client.py @@ -3,7 +3,7 @@ from unittest.mock import call import pytest from app import invite_api_client, service_api_client, user_api_client -from app.notify_client.models import User +from app.models.user import User from tests import sample_uuid from tests.conftest import SERVICE_ONE_ID, api_user_pending @@ -51,7 +51,7 @@ def test_client_returns_count_of_users_with_manage_service( ) mocker.patch( - 'app.notify_client.models._get_service_id_from_view_args', + 'app.models.user._get_service_id_from_view_args', return_value=SERVICE_ONE_ID, ) @@ -221,7 +221,7 @@ def test_returns_value_from_cache( 'app.notify_client.RedisClient.set', ) mock_model = mocker.patch( - 'app.notify_client.models.User.__init__', + 'app.models.user.User.__init__', return_value=None, ) diff --git a/tests/conftest.py b/tests/conftest.py index 45edb97e6..e4a1e5b36 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -11,7 +11,7 @@ from notifications_python_client.errors import HTTPError from notifications_utils.url_safe_token import generate_token from app import create_app -from app.notify_client.models import InvitedOrgUser, InvitedUser, User +from app.models.user import InvitedOrgUser, InvitedUser, User from . import ( TestClient,