darkhelm/notifications-admin
1
0
Fork 0
mirror of https://github.com/GSA/notifications-admin.git synced 2026-09-09 09:39:49 -04:00
Code Issues Packages Projects Releases Wiki Activity

Replace sevice api client get template calls with Service methods

Browse Source 
Instead of using the API client directly views are now calling one
of two Service model methods:

`get_template` is used for view actions, where the user should see
the template page even if they don't have access to the template
folder (since all templates are still inked from the dashboard or
the sent notifications pages).

`get_template_with_user_permission_or_403` will check if the user
has access to the template's folder first and return 403 otherwise.
This method is used for any endpoints that result in an action: editing
template attributes, deleting templates or sending messages.
This commit is contained in:
Alexey Bezhan
2019-03-19 16:25:44 +00:00
parent a30c9733b0
commit 35fb92c02c
5 changed files with 41 additions and 38 deletions
Download Patch File Download Diff File Expand all files Collapse all files

22
app/main/views/send.py
View File

@@ -111,7 +111,7 @@ def send_messages(service_id, template_id):
session['file_uploads'].keys()) session['file_uploads'].keys())
) )
db_template = service_api_client.get_service_template(service_id, template_id)['data'] db_template = current_service.get_template_with_user_permission_or_403(template_id, current_user)
email_reply_to = None email_reply_to = None
sms_sender = None sms_sender = None
@@ -209,7 +209,7 @@ def set_sender(service_id, template_id):
url_for('.send_one_off', service_id=service_id, template_id=template_id) url_for('.send_one_off', service_id=service_id, template_id=template_id)
) )
template = service_api_client.get_service_template(service_id, template_id)['data'] template = current_service.get_template_with_user_permission_or_403(template_id, current_user)
if template['template_type'] == 'letter': if template['template_type'] == 'letter':
return redirect_to_one_off return redirect_to_one_off
@@ -297,7 +297,7 @@ def send_test(service_id, template_id):
session['placeholders'] = {} session['placeholders'] = {}
session['send_test_letter_page_count'] = None session['send_test_letter_page_count'] = None
db_template = service_api_client.get_service_template(service_id, template_id)['data'] db_template = current_service.get_template_with_user_permission_or_403(template_id, current_user)
if db_template['template_type'] == 'letter': if db_template['template_type'] == 'letter':
session['sender_id'] = None session['sender_id'] = None
@@ -354,7 +354,7 @@ def send_test_step(service_id, template_id, step_index):
template_id=template_id, template_id=template_id,
)) ))
db_template = service_api_client.get_service_template(service_id, template_id)['data'] db_template = current_service.get_template_with_user_permission_or_403(template_id, current_user)
if not session.get('send_test_letter_page_count'): if not session.get('send_test_letter_page_count'):
session['send_test_letter_page_count'] = get_page_count_for_letter(db_template) session['send_test_letter_page_count'] = get_page_count_for_letter(db_template)
@@ -483,7 +483,7 @@ def send_test_preview(service_id, template_id, filetype):
if filetype not in ('pdf', 'png'): if filetype not in ('pdf', 'png'):
abort(404) abort(404)
db_template = service_api_client.get_service_template(service_id, template_id)['data'] db_template = current_service.get_template_with_user_permission_or_403(template_id, current_user)
template = get_template( template = get_template(
db_template, db_template,
@@ -528,10 +528,7 @@ def _check_messages(service_id, template_id, upload_id, preview_row, letters_as_
contents = s3download(service_id, upload_id) contents = s3download(service_id, upload_id)
db_template = service_api_client.get_service_template( db_template = current_service.get_template_with_user_permission_or_403(template_id, current_user)
service_id,
str(template_id),
)['data']
email_reply_to = None email_reply_to = None
sms_sender = None sms_sender = None
@@ -843,7 +840,7 @@ def check_notification(service_id, template_id):
def _check_notification(service_id, template_id, exception=None): def _check_notification(service_id, template_id, exception=None):
db_template = service_api_client.get_service_template(service_id, template_id)['data'] db_template = current_service.get_template_with_user_permission_or_403(template_id, current_user)
email_reply_to = None email_reply_to = None
sms_sender = None sms_sender = None
if db_template['template_type'] == 'email': if db_template['template_type'] == 'email':
@@ -917,10 +914,13 @@ def send_notification(service_id, template_id):
service_id=service_id, service_id=service_id,
template_id=template_id, template_id=template_id,
)) ))
db_template = current_service.get_template_with_user_permission_or_403(template_id, current_user)
try: try:
noti = notification_api_client.send_notification( noti = notification_api_client.send_notification(
service_id, service_id,
template_id=template_id, template_id=db_template['id'],
recipient=session['recipient'] or session['placeholders']['address line 1'], recipient=session['recipient'] or session['placeholders']['address line 1'],
personalisation=session['placeholders'], personalisation=session['placeholders'],
sender_id=session['sender_id'] if 'sender_id' in session else None sender_id=session['sender_id'] if 'sender_id' in session else None

27
app/main/views/templates.py
View File

@@ -50,7 +50,8 @@ form_objects = {
@login_required @login_required
@user_has_permissions() @user_has_permissions()
def view_template(service_id, template_id): def view_template(service_id, template_id):
template = service_api_client.get_service_template(service_id, str(template_id))['data'] template = current_service.get_template(template_id)
if should_skip_template_page(template['template_type']): if should_skip_template_page(template['template_type']):
return redirect(url_for( return redirect(url_for(
'.send_one_off', service_id=service_id, template_id=template_id '.send_one_off', service_id=service_id, template_id=template_id
@@ -87,7 +88,7 @@ def view_template(service_id, template_id):
@user_has_permissions('view_activity') @user_has_permissions('view_activity')
def start_tour(service_id, template_id): def start_tour(service_id, template_id):
template = service_api_client.get_service_template(service_id, str(template_id))['data'] template = current_service.get_template(template_id)
if template['template_type'] != 'sms': if template['template_type'] != 'sms':
abort(404) abort(404)
@@ -217,7 +218,7 @@ def view_letter_template_preview(service_id, template_id, filetype):
if filetype not in ('pdf', 'png'): if filetype not in ('pdf', 'png'):
abort(404) abort(404)
db_template = service_api_client.get_service_template(service_id, template_id)['data'] db_template = current_service.get_template(template_id)
return TemplatePreview.from_database_object(db_template, filetype, page=request.args.get('page')) return TemplatePreview.from_database_object(db_template, filetype, page=request.args.get('page'))
@@ -252,7 +253,7 @@ def letter_branding_preview_image(filename):
def _view_template_version(service_id, template_id, version, letters_as_pdf=False): def _view_template_version(service_id, template_id, version, letters_as_pdf=False):
return dict(template=get_template( return dict(template=get_template(
service_api_client.get_service_template(service_id, template_id, version=version)['data'], current_service.get_template(template_id, version=version),
current_service, current_service,
expand_emails=True, expand_emails=True,
letter_preview_url=url_for( letter_preview_url=url_for(
@@ -279,7 +280,7 @@ def view_template_version(service_id, template_id, version):
@login_required @login_required
@user_has_permissions() @user_has_permissions()
def view_template_version_preview(service_id, template_id, version, filetype): def view_template_version_preview(service_id, template_id, version, filetype):
db_template = service_api_client.get_service_template(service_id, template_id, version=version)['data'] db_template = current_service.get_template(template_id, version=version)
return TemplatePreview.from_database_object(db_template, filetype) return TemplatePreview.from_database_object(db_template, filetype)
@@ -567,7 +568,7 @@ def abort_403_if_not_admin_user():
@login_required @login_required
@user_has_permissions('manage_templates') @user_has_permissions('manage_templates')
def edit_service_template(service_id, template_id): def edit_service_template(service_id, template_id):
template = service_api_client.get_service_template(service_id, template_id)['data'] template = current_service.get_template_with_user_permission_or_403(template_id, current_user)
template['template_content'] = template['content'] template['template_content'] = template['content']
form = form_objects[template['template_type']](**template) form = form_objects[template['template_type']](**template)
if form.validate_on_submit(): if form.validate_on_submit():
@@ -629,13 +630,11 @@ def edit_service_template(service_id, template_id):
template_id=template_id template_id=template_id
)) ))
db_template = service_api_client.get_service_template(service_id, template_id)['data'] if email_or_sms_not_enabled(template['template_type'], current_service.permissions):
if email_or_sms_not_enabled(db_template['template_type'], current_service.permissions):
return redirect(url_for( return redirect(url_for(
'.action_blocked', '.action_blocked',
service_id=service_id, service_id=service_id,
notification_type=db_template['template_type'], notification_type=template['template_type'],
return_to='view_template', return_to='view_template',
template_id=template_id template_id=template_id
)) ))
@@ -653,7 +652,7 @@ def edit_service_template(service_id, template_id):
@login_required @login_required
@user_has_permissions('manage_templates') @user_has_permissions('manage_templates')
def delete_service_template(service_id, template_id): def delete_service_template(service_id, template_id):
template = service_api_client.get_service_template(service_id, template_id)['data'] template = current_service.get_template_with_user_permission_or_403(template_id, current_user)
if request.method == 'POST': if request.method == 'POST':
service_api_client.delete_service_template(service_id, template_id) service_api_client.delete_service_template(service_id, template_id)
@@ -702,7 +701,7 @@ def delete_service_template(service_id, template_id):
@login_required @login_required
@user_has_permissions('manage_templates') @user_has_permissions('manage_templates')
def confirm_redact_template(service_id, template_id): def confirm_redact_template(service_id, template_id):
template = service_api_client.get_service_template(service_id, template_id)['data'] template = current_service.get_template_with_user_permission_or_403(template_id, current_user)
return render_template( return render_template(
'views/templates/template.html', 'views/templates/template.html',
@@ -769,7 +768,7 @@ def view_template_versions(service_id, template_id):
@login_required @login_required
@user_has_permissions('manage_templates') @user_has_permissions('manage_templates')
def set_template_sender(service_id, template_id): def set_template_sender(service_id, template_id):
template = service_api_client.get_service_template(service_id, template_id)['data'] template = current_service.get_template_with_user_permission_or_403(template_id, current_user)
sender_details = get_template_sender_form_dict(service_id, template) sender_details = get_template_sender_form_dict(service_id, template)
no_senders = sender_details.get('no_senders', False) no_senders = sender_details.get('no_senders', False)
@@ -800,7 +799,7 @@ def set_template_sender(service_id, template_id):
@login_required @login_required
@user_has_permissions('manage_templates') @user_has_permissions('manage_templates')
def edit_template_postage(service_id, template_id): def edit_template_postage(service_id, template_id):
template = service_api_client.get_service_template(service_id, template_id)['data'] template = current_service.get_template_with_user_permission_or_403(template_id, current_user)
if template["template_type"] != "letter": if template["template_type"] != "letter":
abort(404) abort(404)
form = LetterTemplatePostageForm(**template) form = LetterTemplatePostageForm(**template)

7
tests/app/main/views/test_send.py
View File

@@ -1823,7 +1823,7 @@ def test_send_test_works_as_letter_preview(
) )
) )
mock_get_service_letter_template.assert_called_with(service_id, template_id) mock_get_service_letter_template.assert_called_with(service_id, template_id, None)
assert response.status_code == 200 assert response.status_code == 200
assert response.get_data(as_text=True) == 'foo' assert response.get_data(as_text=True) == 'foo'
@@ -2187,7 +2187,7 @@ def test_should_show_preview_letter_message(
) )
) )
mock_get_service_letter_template.assert_called_with(service_id, template_id) mock_get_service_letter_template.assert_called_with(service_id, template_id, None)
assert response.status_code == 200 assert response.status_code == 200
assert response.get_data(as_text=True) == 'foo' assert response.get_data(as_text=True) == 'foo'
@@ -3121,6 +3121,7 @@ def test_send_notification_submits_data(
client_request, client_request,
fake_uuid, fake_uuid,
mock_send_notification, mock_send_notification,
mock_get_service_template,
template, template,
recipient, recipient,
placeholders, placeholders,
@@ -3150,6 +3151,7 @@ def test_send_notification_clears_session(
service_one, service_one,
fake_uuid, fake_uuid,
mock_send_notification, mock_send_notification,
mock_get_service_template,
): ):
with client_request.session_transaction() as session: with client_request.session_transaction() as session:
session['recipient'] = '07700900001' session['recipient'] = '07700900001'
@@ -3195,6 +3197,7 @@ def test_send_notification_redirects_to_view_page(
client_request, client_request,
fake_uuid, fake_uuid,
mock_send_notification, mock_send_notification,
mock_get_service_template,
extra_args, extra_args,
extra_redirect_args extra_redirect_args
): ):

19
tests/app/main/views/test_templates.py
View File

@@ -347,7 +347,7 @@ def test_should_show_page_for_one_template(
page.select_one('textarea') page.select_one('textarea')
) )
assert "priority" not in str(page.select_one('main')) assert "priority" not in str(page.select_one('main'))
mock_get_service_template.assert_called_with(SERVICE_ONE_ID, template_id) mock_get_service_template.assert_called_with(SERVICE_ONE_ID, template_id, None)
def test_caseworker_redirected_to_one_off( def test_caseworker_redirected_to_one_off(
@@ -703,13 +703,14 @@ def test_should_show_page_template_with_priority_select_if_platform_admin(
platform_admin_user, platform_admin_user,
mocker, mocker,
mock_get_service_template, mock_get_service_template,
service_one,
fake_uuid, fake_uuid,
): ):
mocker.patch('app.user_api_client.get_users_for_service', return_value=[platform_admin_user]) mocker.patch('app.user_api_client.get_users_for_service', return_value=[platform_admin_user])
template_id = fake_uuid template_id = fake_uuid
response = logged_in_platform_admin_client.get(url_for( response = logged_in_platform_admin_client.get(url_for(
'.edit_service_template', '.edit_service_template',
service_id='1234', service_id=service_one['id'],
template_id=template_id, template_id=template_id,
)) ))
@@ -717,7 +718,7 @@ def test_should_show_page_template_with_priority_select_if_platform_admin(
assert "Two week reminder" in response.get_data(as_text=True) assert "Two week reminder" in response.get_data(as_text=True)
assert "Template <em>content</em> with & entity" in response.get_data(as_text=True) assert "Template <em>content</em> with & entity" in response.get_data(as_text=True)
assert "Use priority queue?" in response.get_data(as_text=True) assert "Use priority queue?" in response.get_data(as_text=True)
mock_get_service_template.assert_called_with('1234', template_id) mock_get_service_template.assert_called_with(service_one['id'], template_id, None)
@pytest.mark.parametrize('filetype', ['pdf', 'png']) @pytest.mark.parametrize('filetype', ['pdf', 'png'])
@@ -752,7 +753,7 @@ def test_should_show_preview_letter_templates(
assert response.status_code == 200 assert response.status_code == 200
assert response.get_data(as_text=True) == 'foo' assert response.get_data(as_text=True) == 'foo'
mock_get_service_email_template.assert_called_with(service_id, template_id, **extra_view_args) mock_get_service_email_template.assert_called_with(service_id, template_id, extra_view_args.get('version'))
assert mocked_preview.call_args[0][0]['id'] == template_id assert mocked_preview.call_args[0][0]['id'] == template_id
assert mocked_preview.call_args[0][0]['service'] == service_id assert mocked_preview.call_args[0][0]['service'] == service_id
assert mocked_preview.call_args[0][1] == filetype assert mocked_preview.call_args[0][1] == filetype
@@ -1661,7 +1662,7 @@ def test_should_show_delete_template_page_with_time_block(
assert normalize_spaces(page.select('.sms-message-wrapper')[0].text) == ( assert normalize_spaces(page.select('.sms-message-wrapper')[0].text) == (
'service one: Template <em>content</em> with & entity' 'service one: Template <em>content</em> with & entity'
) )
mock_get_service_template.assert_called_with(SERVICE_ONE_ID, fake_uuid) mock_get_service_template.assert_called_with(SERVICE_ONE_ID, fake_uuid, None)
def test_should_show_delete_template_page_with_time_block_for_empty_notification( def test_should_show_delete_template_page_with_time_block_for_empty_notification(
@@ -1691,7 +1692,7 @@ def test_should_show_delete_template_page_with_time_block_for_empty_notification
assert normalize_spaces(page.select('.sms-message-wrapper')[0].text) == ( assert normalize_spaces(page.select('.sms-message-wrapper')[0].text) == (
'service one: Template <em>content</em> with & entity' 'service one: Template <em>content</em> with & entity'
) )
mock_get_service_template.assert_called_with(SERVICE_ONE_ID, fake_uuid) mock_get_service_template.assert_called_with(SERVICE_ONE_ID, fake_uuid, None)
def test_should_show_delete_template_page_with_never_used_block( def test_should_show_delete_template_page_with_never_used_block(
@@ -1716,7 +1717,7 @@ def test_should_show_delete_template_page_with_never_used_block(
assert normalize_spaces(page.select('.sms-message-wrapper')[0].text) == ( assert normalize_spaces(page.select('.sms-message-wrapper')[0].text) == (
'service one: Template <em>content</em> with & entity' 'service one: Template <em>content</em> with & entity'
) )
mock_get_service_template.assert_called_with(SERVICE_ONE_ID, fake_uuid) mock_get_service_template.assert_called_with(SERVICE_ONE_ID, fake_uuid, None)
@pytest.mark.parametrize('parent', ( @pytest.mark.parametrize('parent', (
@@ -1749,7 +1750,7 @@ def test_should_redirect_when_deleting_a_template(
) )
mock_get_service_template.assert_called_with( mock_get_service_template.assert_called_with(
SERVICE_ONE_ID, TEMPLATE_ONE_ID SERVICE_ONE_ID, TEMPLATE_ONE_ID, None
) )
mock_delete_service_template.assert_called_with( mock_delete_service_template.assert_called_with(
SERVICE_ONE_ID, TEMPLATE_ONE_ID SERVICE_ONE_ID, TEMPLATE_ONE_ID
@@ -1780,7 +1781,7 @@ def test_should_show_page_for_a_deleted_template(
assert page.select('p.hint')[0].text.strip() == 'This template was deleted today at 3:00pm.' assert page.select('p.hint')[0].text.strip() == 'This template was deleted today at 3:00pm.'
assert 'Delete this template' not in page.select_one('main').text assert 'Delete this template' not in page.select_one('main').text
mock_get_deleted_template.assert_called_with(SERVICE_ONE_ID, template_id) mock_get_deleted_template.assert_called_with(SERVICE_ONE_ID, template_id, None)
@pytest.mark.parametrize('route', [ @pytest.mark.parametrize('route', [

4
tests/conftest.py
View File

@@ -835,7 +835,7 @@ def mock_get_template_versions(mocker, fake_uuid, user=None):
@pytest.fixture(scope='function') @pytest.fixture(scope='function')
def mock_get_service_template_with_placeholders(mocker): def mock_get_service_template_with_placeholders(mocker):
def _get(service_id, template_id): def _get(service_id, template_id, version=None):
template = template_json( template = template_json(
service_id, template_id, "Two week reminder", "sms", "((name)), Template <em>content</em> with & entity" service_id, template_id, "Two week reminder", "sms", "((name)), Template <em>content</em> with & entity"
) )
@@ -849,7 +849,7 @@ def mock_get_service_template_with_placeholders(mocker):
@pytest.fixture(scope='function') @pytest.fixture(scope='function')
def mock_get_service_template_with_placeholders_same_as_recipient(mocker): def mock_get_service_template_with_placeholders_same_as_recipient(mocker):
def _get(service_id, template_id): def _get(service_id, template_id, version=None):
template = template_json( template = template_json(
service_id, template_id, "Two week reminder", "sms", "((name)) ((date)) ((PHONENUMBER))" service_id, template_id, "Two week reminder", "sms", "((name)) ((date)) ((PHONENUMBER))"
) )