Don’t allow indexing on service model

Making people use a property is a sure way to make sure they’re spelling
the name of the property correctly, and allows us to easily swap out
properties that call through to the underlying JSON, and properties
which are implemented as methods.
This commit is contained in:
Chris Hill-Scott
2018-10-26 12:13:04 +01:00
parent 780b9bb715
commit e1197c54a5
10 changed files with 17 additions and 12 deletions

View File

@@ -324,7 +324,7 @@ def get_status_filters(service, message_type, statistics):
option,
url_for(
'.view_notifications',
service_id=service['id'],
service_id=service.id,
message_type=message_type,
status=option
),

View File

@@ -346,7 +346,7 @@ def service_switch_can_upload_document(service_id):
# If turning the permission off, or turning it on and the service already has a contact_link,
# don't show the form to add the link
if current_service.has_permission('upload_document') or current_service.get('contact_link'):
if current_service.has_permission('upload_document') or current_service.contact_link:
switch_service_permissions(service_id, 'upload_document')
return redirect(url_for('.service_settings', service_id=service_id))
@@ -415,7 +415,7 @@ def service_set_contact_link(service_id):
form = ServiceContactDetailsForm()
if request.method == 'GET':
contact_details = current_service.get('contact_link')
contact_details = current_service.contact_link
contact_type = check_contact_details_type(contact_details)
field_to_update = getattr(form, contact_type)

View File

@@ -237,7 +237,7 @@ def choose_template_to_copy(service_id):
'templates': [
template for template in
service_api_client.get_service_templates(service['id'])['data']
if template['template_type'] in current_service['permissions']
if current_service.has_permission(template['template_type'])
],
} for service in user_api_client.get_services_for_user(current_user)],
)

View File

@@ -273,6 +273,7 @@ class Service():
ALLOWED_PROPERTIES = {
'active',
'contact_link',
'dvla_organisation',
'email_branding',
'email_from',
@@ -305,7 +306,9 @@ class Service():
raise AttributeError('`{}` is not a service attribute'.format(attr))
def __getitem__(self, attr):
return self.__getattr__(attr)
raise NotImplementedError(
'Use current_service.{} instead of current_service[\'{}\']'.format(attr, attr)
)
def get(self, attr, default=None):
try:

View File

@@ -17,7 +17,7 @@
{% for service in organisation_services %}
<li class="browse-list-item">
{% if service.has_permission_to_view %}
<a href="{{ url_for('main.service_dashboard', service_id=service['id']) }}" class="browse-list-link">{{ service['name'] }}</a>
<a href="{{ url_for('main.service_dashboard', service_id=service.id) }}" class="browse-list-link">{{ service['name'] }}</a>
{% else %}
{{ service['name'] }}
{% endif %}

View File

@@ -58,7 +58,7 @@
<p>
Set up separate email addresses to receive replies
from your users.
{% if current_service.restricted and not reply_to_email_addresses %}
{% if current_service.trial_mode and not reply_to_email_addresses %}
Your service cant go live until youve added at least one
reply-to address.
{% endif %}

View File

@@ -22,7 +22,7 @@
Set up a separate email address to receive replies from
your users, then enter it here.
</p>
{% if current_service.restricted %}
{% if current_service.trial_mode %}
<p>
Your service cant go live until youve done this.
</p>

View File

@@ -19,7 +19,7 @@
<p>When you create a GOV.UK Notify account, youll start in trial mode. This lets you try out the service, but has some restrictions in place.</p>
<p>
You can remove these restrictions by
{% if current_service and current_service.restricted %}
{% if current_service and current_service.trial_mode %}
<a href="{{ url_for('.request_to_go_live', service_id=current_service.id) }}">requesting to go live</a>.
{% else %}
going live.

View File

@@ -176,6 +176,7 @@ def service_json(
'service_callback_api': service_callback_api,
'prefix_sms': prefix_sms,
'postage': postage,
'contact_link': None,
}

View File

@@ -9,6 +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 tests.conftest import (
SERVICE_ONE_ID,
active_caseworking_user,
@@ -438,7 +439,7 @@ STATISTICS = {
def test_get_status_filters_calculates_stats(client):
ret = get_status_filters({'id': 'foo'}, 'sms', STATISTICS)
ret = get_status_filters(Service({'id': 'foo'}), 'sms', STATISTICS)
assert {label: count for label, _option, _link, count in ret} == {
'total': 6,
@@ -449,7 +450,7 @@ def test_get_status_filters_calculates_stats(client):
def test_get_status_filters_in_right_order(client):
ret = get_status_filters({'id': 'foo'}, 'sms', STATISTICS)
ret = get_status_filters(Service({'id': 'foo'}), 'sms', STATISTICS)
assert [label for label, _option, _link, _count in ret] == [
'total', 'sending', 'delivered', 'failed'
@@ -457,7 +458,7 @@ def test_get_status_filters_in_right_order(client):
def test_get_status_filters_constructs_links(client):
ret = get_status_filters({'id': 'foo'}, 'sms', STATISTICS)
ret = get_status_filters(Service({'id': 'foo'}), 'sms', STATISTICS)
link = ret[0][2]
assert link == '/services/foo/notifications/sms?status={}'.format(quote('sending,delivered,failed'))