From 22fe164711c1518be909c6e624a5d24a7b337246 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Tue, 26 Apr 2016 10:32:26 +0100 Subject: [PATCH 1/3] Move feedback endpoints into own file MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Doesn’t make sense to shove more stuff into index.py --- app/main/__init__.py | 3 +- app/main/views/feedback.py | 42 +++++++++++ app/main/views/index.py | 39 +--------- tests/app/main/views/test_feedback.py | 101 ++++++++++++++++++++++++++ tests/app/main/views/test_index.py | 86 +--------------------- 5 files changed, 147 insertions(+), 124 deletions(-) create mode 100644 app/main/views/feedback.py create mode 100644 tests/app/main/views/test_feedback.py diff --git a/app/main/__init__.py b/app/main/__init__.py index 4475482c1..adda9f715 100644 --- a/app/main/__init__.py +++ b/app/main/__init__.py @@ -24,5 +24,6 @@ from app.main.views import ( manage_users, invites, all_services, - tour + tour, + feedback ) diff --git a/app/main/views/feedback.py b/app/main/views/feedback.py new file mode 100644 index 000000000..38804b973 --- /dev/null +++ b/app/main/views/feedback.py @@ -0,0 +1,42 @@ +import markdown +import requests +from flask import render_template, url_for, redirect, flash, current_app, abort +from app.main import main +from flask_login import login_required +from app.main.forms import Feedback + +from flask.ext.login import current_user + + +@main.route('/feedback', methods=['GET', 'POST']) +def feedback(): + form = Feedback() + if form.validate_on_submit(): + data = { + 'person_email': current_app.config.get('DESKPRO_PERSON_EMAIL'), + 'department_id': current_app.config.get('DESKPRO_TEAM_ID'), + 'subject': 'Notify feedback', + 'message': '{}\n{}\n{}'.format( + form.name.data, + form.email_address.data, + form.feedback.data) + } + headers = { + "X-DeskPRO-API-Key": current_app.config.get('DESKPRO_API_KEY'), + 'Content-Type': "application/x-www-form-urlencoded" + } + resp = requests.post( + current_app.config.get('DESKPRO_API_HOST') + '/api/tickets', + data=data, + headers=headers) + if resp.status_code != 201: + current_app.logger.error( + "Deskpro create ticket request failed with {} '{}'".format( + resp.status_code, + resp.json()) + ) + abort(500, "Feedback submission failed") + flash("Your feedback has been submitted") + return redirect(url_for('.feedback')) + + return render_template('views/feedback.html', form=form) \ No newline at end of file diff --git a/app/main/views/index.py b/app/main/views/index.py index 198c77913..2c4b7135b 100644 --- a/app/main/views/index.py +++ b/app/main/views/index.py @@ -1,11 +1,8 @@ import markdown import os -import requests -import json -from flask import (render_template, url_for, redirect, Markup, flash, current_app, abort) +from flask import (render_template, url_for, redirect, Markup, current_app, abort) from app.main import main from flask_login import login_required -from app.main.forms import Feedback from flask.ext.login import current_user from mdx_gfm import GithubFlavoredMarkdownExtension @@ -44,40 +41,6 @@ def terms(): return render_template('views/terms-of-use.html') -@main.route('/feedback', methods=['GET', 'POST']) -def feedback(): - form = Feedback() - if form.validate_on_submit(): - data = { - 'person_email': current_app.config.get('DESKPRO_PERSON_EMAIL'), - 'department_id': current_app.config.get('DESKPRO_TEAM_ID'), - 'subject': 'Notify feedback', - 'message': '{}\n{}\n{}'.format( - form.name.data, - form.email_address.data, - form.feedback.data) - } - headers = { - "X-DeskPRO-API-Key": current_app.config.get('DESKPRO_API_KEY'), - 'Content-Type': "application/x-www-form-urlencoded" - } - resp = requests.post( - current_app.config.get('DESKPRO_API_HOST') + '/api/tickets', - data=data, - headers=headers) - if resp.status_code != 201: - current_app.logger.error( - "Deskpro create ticket request failed with {} '{}'".format( - resp.status_code, - resp.json()) - ) - abort(500, "Feedback submission failed") - flash("Your feedback has been submitted") - return redirect(url_for('.feedback')) - - return render_template('views/feedback.html', form=form) - - @main.route('/documentation') def documentation(): curr_dir = os.path.dirname(os.path.realpath(__file__)) diff --git a/tests/app/main/views/test_feedback.py b/tests/app/main/views/test_feedback.py new file mode 100644 index 000000000..12a654089 --- /dev/null +++ b/tests/app/main/views/test_feedback.py @@ -0,0 +1,101 @@ +import pytest +from flask import (url_for, current_app) +from werkzeug.exceptions import InternalServerError +from unittest.mock import Mock, ANY + + +def test_logged_in_user_redirects_to_choose_service(app_, + api_user_active, + mock_get_user, + mock_get_user_by_email, + mock_login): + with app_.test_request_context(): + with app_.test_client() as client: + client.login(api_user_active) + response = client.get(url_for('main.index')) + assert response.status_code == 302 + + response = client.get(url_for('main.sign_in', follow_redirects=True)) + assert response.location == url_for('main.choose_service', _external=True) + + +def test_get_feedback_page(app_): + with app_.test_request_context(): + with app_.test_client() as client: + resp = client.get(url_for('main.feedback')) + assert resp.status_code == 200 + + +def test_post_feedback_with_no_name_email(app_, mocker): + mock_post = mocker.patch( + 'app.main.views.feedback.requests.post', + return_value=Mock(status_code=201)) + with app_.test_request_context(): + with app_.test_client() as client: + resp = client.post(url_for('main.feedback'), data={'feedback': "blah"}) + assert resp.status_code == 302 + + +def test_post_feedback_with_no_name_email(app_, mocker): + mock_post = mocker.patch( + 'app.main.views.feedback.requests.post', + return_value=Mock(status_code=201)) + with app_.test_request_context(): + with app_.test_client() as client: + resp = client.post(url_for('main.feedback'), data={'feedback': "blah"}) + assert resp.status_code == 302 + mock_post.assert_called_with( + ANY, + data={ + 'department_id': ANY, + 'subject': 'Notify feedback', + 'message': '\n\nblah', + 'person_email': ANY}, + headers=ANY) + + +def test_post_feedback_with_name_email(app_, mocker): + mock_post = mocker.patch( + 'app.main.views.feedback.requests.post', + return_value=Mock(status_code=201)) + with app_.test_request_context(): + with app_.test_client() as client: + resp = client.post( + url_for('main.feedback'), + data={'feedback': "blah", 'name': "Steve Irwin", 'email_address': 'rip@gmail.com'}) + assert resp.status_code == 302 + mock_post.assert_called_with( + ANY, + data={ + 'subject': 'Notify feedback', + 'department_id': ANY, + 'message': 'Steve Irwin\nrip@gmail.com\nblah', + 'person_email': ANY}, + headers=ANY) + + +def test_log_error_on_post(app_, mocker): + mock_post = mocker.patch( + 'app.main.views.feedback.requests.post', + return_value=Mock( + status_code=401, + json=lambda: { + 'error_code': 'invalid_auth', + 'error_message': 'Please provide a valid API key or token'})) + with app_.test_request_context(): + mock_logger = mocker.patch.object(app_.logger, 'error') + with app_.test_client() as client: + with pytest.raises(InternalServerError): + resp = client.post( + url_for('main.feedback'), + data={'feedback': "blah", 'name': "Steve Irwin", 'email_address': 'rip@gmail.com'}) + mock_post.assert_called_with( + ANY, + data={ + 'subject': 'Notify feedback', + 'department_id': ANY, + 'message': 'Steve Irwin\nrip@gmail.com\nblah', + 'person_email': ANY}, + headers=ANY) + mock_logger.assert_called_with( + "Deskpro create ticket request failed with {} '{}'".format(mock_post().status_code, mock_post().json())) diff --git a/tests/app/main/views/test_index.py b/tests/app/main/views/test_index.py index 64c93db48..51ff612d3 100644 --- a/tests/app/main/views/test_index.py +++ b/tests/app/main/views/test_index.py @@ -1,7 +1,5 @@ import pytest -from flask import (url_for, current_app) -from werkzeug.exceptions import InternalServerError -from unittest.mock import Mock, ANY +from flask import url_for def test_logged_in_user_redirects_to_choose_service(app_, @@ -17,85 +15,3 @@ def test_logged_in_user_redirects_to_choose_service(app_, response = client.get(url_for('main.sign_in', follow_redirects=True)) assert response.location == url_for('main.choose_service', _external=True) - - -def test_get_feedback_page(app_): - with app_.test_request_context(): - with app_.test_client() as client: - resp = client.get(url_for('main.feedback')) - assert resp.status_code == 200 - - -def test_post_feedback_with_no_name_email(app_, mocker): - mock_post = mocker.patch( - 'app.main.views.index.requests.post', - return_value=Mock(status_code=201)) - with app_.test_request_context(): - with app_.test_client() as client: - resp = client.post(url_for('main.feedback'), data={'feedback': "blah"}) - assert resp.status_code == 302 - - -def test_post_feedback_with_no_name_email(app_, mocker): - mock_post = mocker.patch( - 'app.main.views.index.requests.post', - return_value=Mock(status_code=201)) - with app_.test_request_context(): - with app_.test_client() as client: - resp = client.post(url_for('main.feedback'), data={'feedback': "blah"}) - assert resp.status_code == 302 - mock_post.assert_called_with( - ANY, - data={ - 'department_id': ANY, - 'subject': 'Notify feedback', - 'message': '\n\nblah', - 'person_email': ANY}, - headers=ANY) - - -def test_post_feedback_with_name_email(app_, mocker): - mock_post = mocker.patch( - 'app.main.views.index.requests.post', - return_value=Mock(status_code=201)) - with app_.test_request_context(): - with app_.test_client() as client: - resp = client.post( - url_for('main.feedback'), - data={'feedback': "blah", 'name': "Steve Irwin", 'email_address': 'rip@gmail.com'}) - assert resp.status_code == 302 - mock_post.assert_called_with( - ANY, - data={ - 'subject': 'Notify feedback', - 'department_id': ANY, - 'message': 'Steve Irwin\nrip@gmail.com\nblah', - 'person_email': ANY}, - headers=ANY) - - -def test_log_error_on_post(app_, mocker): - mock_post = mocker.patch( - 'app.main.views.index.requests.post', - return_value=Mock( - status_code=401, - json=lambda: { - 'error_code': 'invalid_auth', - 'error_message': 'Please provide a valid API key or token'})) - with app_.test_request_context(): - mock_logger = mocker.patch.object(app_.logger, 'error') - with app_.test_client() as client: - with pytest.raises(InternalServerError): - resp = client.post( - url_for('main.feedback'), - data={'feedback': "blah", 'name': "Steve Irwin", 'email_address': 'rip@gmail.com'}) - mock_post.assert_called_with( - ANY, - data={ - 'subject': 'Notify feedback', - 'department_id': ANY, - 'message': 'Steve Irwin\nrip@gmail.com\nblah', - 'person_email': ANY}, - headers=ANY) - mock_logger.assert_called_with( - "Deskpro create ticket request failed with {} '{}'".format(mock_post().status_code, mock_post().json())) From 45241d6232d9458ae8b586d91c374c98f34de471 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Tue, 26 Apr 2016 13:31:57 +0100 Subject: [PATCH 2/3] Put a form on the request to go live page MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This commit: - moves things around a bit on the request to go live page - sticks a textbox in there So when someone click the big green button, we will get a support ticket that looks something like: ``` From Test User on behalf of Test Service (6ce466d0-fd6a-11e5-82f5-e0accb9d11a6) --- We’ll send about 1000 text messages in the first month, and then 10,000 text messages per month after that. Usage of our service is about 50% higher in March, at the end of the tax year. ``` --- app/main/forms.py | 7 ++ app/main/views/feedback.py | 2 +- app/main/views/service_settings.py | 49 ++++++++-- app/templates/components/textbox.html | 7 +- app/templates/views/service-settings.html | 2 +- .../service-settings/request-to-go-live.html | 54 +++++++++-- tests/app/main/views/test_service_settings.py | 90 +++++++++++++------ 7 files changed, 167 insertions(+), 44 deletions(-) diff --git a/app/main/forms.py b/app/main/forms.py index 48fe081f5..81bcb869d 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -313,3 +313,10 @@ class Feedback(Form): name = StringField('Name') email_address = StringField('Email address') feedback = TextAreaField(u'', validators=[DataRequired(message="Can’t be empty")]) + + +class RequestToGoLiveForm(Form): + usage = TextAreaField( + '', + validators=[DataRequired(message="Can’t be empty")] + ) diff --git a/app/main/views/feedback.py b/app/main/views/feedback.py index 38804b973..3c8a1f080 100644 --- a/app/main/views/feedback.py +++ b/app/main/views/feedback.py @@ -39,4 +39,4 @@ def feedback(): flash("Your feedback has been submitted") return redirect(url_for('.feedback')) - return render_template('views/feedback.html', form=form) \ No newline at end of file + return render_template('views/feedback.html', form=form) diff --git a/app/main/views/service_settings.py b/app/main/views/service_settings.py index ef8cb6f95..7d68ea6a2 100644 --- a/app/main/views/service_settings.py +++ b/app/main/views/service_settings.py @@ -1,10 +1,13 @@ +import requests from flask import ( render_template, redirect, request, url_for, session, - flash + flash, + abort, + current_app ) from flask_login import ( @@ -16,7 +19,7 @@ from notifications_python_client.errors import HTTPError from app import service_api_client from app.main import main from app.utils import user_has_permissions, email_safe -from app.main.forms import ConfirmPasswordForm, ServiceNameForm +from app.main.forms import ConfirmPasswordForm, ServiceNameForm, RequestToGoLiveForm from app import user_api_client from app import current_service @@ -86,15 +89,45 @@ def service_name_change_confirm(service_id): @login_required @user_has_permissions('manage_settings', admin_override=True) def service_request_to_go_live(service_id): - if request.method == 'GET': - return render_template( - 'views/service-settings/request-to-go-live.html' + + form = RequestToGoLiveForm() + + if form.validate_on_submit(): + + data = { + 'person_email': current_app.config.get('DESKPRO_PERSON_EMAIL'), + 'department_id': current_app.config.get('DESKPRO_TEAM_ID'), + 'subject': 'Request to go live', + 'message': "From {} <{}> on behalf of {} ({})\n\nUsage estimate\n---\n\n{}".format( + current_user.name, + current_user.email_address, + current_service['name'], + current_service['id'], + form.usage.data + ) + } + headers = { + "X-DeskPRO-API-Key": current_app.config.get('DESKPRO_API_KEY'), + 'Content-Type': "application/x-www-form-urlencoded" + } + resp = requests.post( + current_app.config.get('DESKPRO_API_HOST') + '/api/tickets', + data=data, + headers=headers ) - elif request.method == 'POST': - flash('Thanks your request to go live is being processed', 'default') - # TODO implement whatever this action would do in the real world + if resp.status_code != 201: + current_app.logger.error( + "Deskpro create ticket request failed with {} '{}'".format( + resp.status_code, + resp.json()) + ) + abort(500, "Request to go live submission failed") + + flash('We’ve received your request to go live', 'default') return redirect(url_for('.service_settings', service_id=service_id)) + return render_template('views/service-settings/request-to-go-live.html', form=form) + @main.route("/services//service-settings/switch-live") @login_required diff --git a/app/templates/components/textbox.html b/app/templates/components/textbox.html index e849a6bce..40d141072 100644 --- a/app/templates/components/textbox.html +++ b/app/templates/components/textbox.html @@ -1,5 +1,6 @@ {% macro textbox( field, + label=None, hint=False, highlight_tags=False, autofocus=False, @@ -12,7 +13,11 @@ ) %}