From 5ae9d19014e67e530cb6646ef7a33863d60e173e Mon Sep 17 00:00:00 2001 From: Rob Young Date: Fri, 9 Oct 2015 16:35:31 +0100 Subject: [PATCH 1/8] Upgrade dmutils to 10.0.0 Only change required is around the ContentLoader --- app/__init__.py | 6 ++---- app/main/views/services.py | 10 +++++----- requirements.txt | 2 +- 3 files changed, 8 insertions(+), 10 deletions(-) diff --git a/app/__init__.py b/app/__init__.py index a21d26636..f515b9b2f 100644 --- a/app/__init__.py +++ b/app/__init__.py @@ -18,10 +18,8 @@ feature_flags = flask_featureflags.FeatureFlag() login_manager = LoginManager() -service_content = ContentLoader( - "app/content/frameworks/g-cloud-6/manifests/edit_service_as_admin.yml", - "app/content/frameworks/g-cloud-6/questions/services/" -) +content_loader = ContentLoader('app/content') +content_loader.load_manifest('g-cloud-6', 'edit_service_as_admin', 'services') from app.main.helpers.service import parse_document_upload_time diff --git a/app/main/views/services.py b/app/main/views/services.py index 5655bc0bc..3aba200df 100644 --- a/app/main/views/services.py +++ b/app/main/views/services.py @@ -11,7 +11,7 @@ from ... import data_api_client -from ... import service_content +from ... import content_loader from .. import main from . import get_template_data @@ -51,7 +51,7 @@ def view(service_id): return redirect(url_for('.index')) service_data['priceString'] = format_service_price(service_data) - content = service_content.get_builder().filter(service_data) + content = content_loader.get_builder('g-cloud-6', 'edit_service_as_admin') template_data = get_template_data( sections=content, @@ -101,7 +101,7 @@ def update_service_status(service_id): def edit(service_id, section): service_data = data_api_client.get_service(service_id)['services'] - content = service_content.get_builder().filter(service_data) + content = content_loader.get_builder('g-cloud-6', 'edit_service_as_admin').filter(service_data) template_data = get_template_data( section=content.get_section(section), @@ -153,7 +153,7 @@ def validate_archived_services(old_archived_service, new_archived_service): except (HTTPError, KeyError, ValueError): return abort(404) - content = service_content.get_builder().filter(service_data) + content = content_loader.get_builder('g-cloud-6', 'edit_service_as_admin').filter(service_data) # It's possible to have an empty array if none of the lines were changed. # TODO This possibility isn't actually handled. @@ -188,7 +188,7 @@ def update(service_id, section_id): abort(404) service = service['services'] - content = service_content.get_builder().filter(service) + content = content_loader.get_builder('g-cloud-6', 'edit_service_as_admin').filter(service) section = content.get_section(section_id) if section is None or not section.editable: abort(404) diff --git a/requirements.txt b/requirements.txt index 0bf58852b..ad11db300 100644 --- a/requirements.txt +++ b/requirements.txt @@ -10,7 +10,7 @@ six==1.9.0 boto==2.36.0 markdown==2.6.2 -git+https://github.com/alphagov/digitalmarketplace-utils.git@8.6.0#egg=digitalmarketplace-utils==8.6.0 +git+https://github.com/alphagov/digitalmarketplace-utils.git@10.0.0#egg=digitalmarketplace-utils==10.0.0 # Required for SNI to work in requests pyOpenSSL==0.14 From a504128817226a6dbcad6bb99f01f8b765bdf31b Mon Sep 17 00:00:00 2001 From: Rob Young Date: Fri, 9 Oct 2015 16:37:40 +0100 Subject: [PATCH 2/8] Add ability to edit supplier declarations This adds the ability for a CCS Sourcing team member to view and edit G-Cloud 7 supplier declarations. In the supplier listing the only link they will see alongside a supplier is 'G-Cloud 7 declaration'. This will take them to the G-Cloud 7 supplier declaration overview where they can edit each section. This uses the new ContentLoader but hard codes the G-Cloud 7 link in the supplier list as it is the only framework they can edit at the moment. Note that this does no validation, it is assumed that the CCS Sourcing team know the rules. --- app/__init__.py | 1 + app/assets/scss/application.scss | 1 + app/assets/scss/views/_view_declaration.scss | 3 + app/main/views/suppliers.py | 72 +++++++++++++++++-- app/templates/index.html | 2 +- app/templates/suppliers/edit_declaration.html | 46 ++++++++++++ app/templates/suppliers/view_declaration.html | 44 ++++++++++++ app/templates/view_suppliers.html | 36 +++++++--- 8 files changed, 189 insertions(+), 16 deletions(-) create mode 100644 app/assets/scss/views/_view_declaration.scss create mode 100644 app/templates/suppliers/edit_declaration.html create mode 100644 app/templates/suppliers/view_declaration.html diff --git a/app/__init__.py b/app/__init__.py index f515b9b2f..c16390e78 100644 --- a/app/__init__.py +++ b/app/__init__.py @@ -20,6 +20,7 @@ content_loader = ContentLoader('app/content') content_loader.load_manifest('g-cloud-6', 'edit_service_as_admin', 'services') +content_loader.load_manifest('g-cloud-7', 'declaration', 'declaration') from app.main.helpers.service import parse_document_upload_time diff --git a/app/assets/scss/application.scss b/app/assets/scss/application.scss index c78e8be67..8cfa4c7b7 100644 --- a/app/assets/scss/application.scss +++ b/app/assets/scss/application.scss @@ -37,6 +37,7 @@ $path: "/admin/static/images/"; @import "views/view_audits"; @import "_diff.scss"; @import "views/statistics"; +@import "views/view_declaration"; // Graphs @import "_c3"; diff --git a/app/assets/scss/views/_view_declaration.scss b/app/assets/scss/views/_view_declaration.scss new file mode 100644 index 000000000..4c0b62704 --- /dev/null +++ b/app/assets/scss/views/_view_declaration.scss @@ -0,0 +1,3 @@ +.my-special-page thead .summary-item-field-heading-first { + width: 60%; +} diff --git a/app/main/views/suppliers.py b/app/main/views/suppliers.py index 48f184e99..20176b950 100644 --- a/app/main/views/suppliers.py +++ b/app/main/views/suppliers.py @@ -3,18 +3,17 @@ from .. import main from . import get_template_data -from ... import data_api_client +from ... import data_api_client, content_loader from ..forms import EmailAddressForm from ..auth import role_required -from dmutils.apiclient.errors import HTTPError +from dmutils.apiclient.errors import HTTPError, APIError from dmutils.audit import AuditTypes -from dmutils.email import send_email, \ - generate_token, MandrillException +from dmutils.email import send_email, generate_token, MandrillException @main.route('/suppliers', methods=['GET']) @login_required -@role_required('admin', 'admin-ccs-category') +@role_required('admin', 'admin-ccs-category', 'admin-ccs-sourcing') def find_suppliers(): suppliers = data_api_client.find_suppliers(prefix=request.args.get("supplier_name_prefix")) @@ -39,6 +38,69 @@ def edit_supplier_name(supplier_id): ) +@main.route('/suppliers//edit/declarations/', methods=['GET']) +@login_required +@role_required('admin-ccs-sourcing') +def view_supplier_declaration(supplier_id, framework_slug): + supplier = data_api_client.get_supplier(supplier_id)['suppliers'] + framework = data_api_client.get_framework(framework_slug)['frameworks'] + declaration = data_api_client.get_supplier_declaration(supplier_id, framework_slug)['declaration'] + + content = content_loader.get_builder(framework_slug, 'declaration').filter(declaration) + + return render_template( + "suppliers/view_declaration.html", + supplier=supplier, + framework=framework, + declaration=declaration, + content=content, + **get_template_data()) + + +@main.route( + '/suppliers//edit/declarations//', + methods=['GET']) +@login_required +@role_required('admin-ccs-sourcing') +def edit_supplier_declaration_section(supplier_id, framework_slug, section_id): + supplier = data_api_client.get_supplier(supplier_id)['suppliers'] + framework = data_api_client.get_framework(framework_slug)['frameworks'] + declaration = data_api_client.get_supplier_declaration(supplier_id, framework_slug)['declaration'] + + content = content_loader.get_builder(framework_slug, 'declaration').filter(declaration) + + return render_template( + "suppliers/edit_declaration.html", + supplier=supplier, + framework=framework, + declaration=declaration, + section=content.get_section(section_id), + **get_template_data()) + + +@main.route( + '/suppliers//edit/declarations//', + methods=['POST']) +def update_supplier_declaration_section(supplier_id, framework_slug, section_id): + supplier = data_api_client.get_supplier(supplier_id)['suppliers'] + framework = data_api_client.get_framework(framework_slug)['frameworks'] + declaration = data_api_client.get_supplier_declaration(supplier_id, framework_slug)['declaration'] + + content = content_loader.get_builder(framework_slug, 'declaration').filter(declaration) + section = content.get_section(section_id) + + posted_data = section.get_data(request.form) + + if section.has_changes_to_save(declaration, posted_data): + declaration.update(posted_data) + data_api_client.set_supplier_declaration( + supplier_id, framework_slug, declaration, + current_user.email_address) + + return redirect(url_for('.view_supplier_declaration', + supplier_id=supplier_id, framework_slug=framework_slug)) + + @main.route('/suppliers//edit/name', methods=['POST']) @login_required @role_required('admin') diff --git a/app/templates/index.html b/app/templates/index.html index d1e3af32a..b14cb7e3e 100644 --- a/app/templates/index.html +++ b/app/templates/index.html @@ -32,7 +32,7 @@ {% include "toolkit/page-heading.html" %} {% endwith %} - {% if current_user.has_role('admin') or current_user.has_role('admin-ccs-category') %} + {% if current_user.has_any_role('admin', 'admin-ccs-category', 'admin-ccs-sourcing') %}
diff --git a/app/templates/suppliers/edit_declaration.html b/app/templates/suppliers/edit_declaration.html new file mode 100644 index 000000000..adbc99e0c --- /dev/null +++ b/app/templates/suppliers/edit_declaration.html @@ -0,0 +1,46 @@ +{% import 'macros/toolkit_forms.html' as forms %} + +{% extends "_base_page.html" %} + +{% block page_title %} + Change {{ framework.name }} declaration +{% endblock %} + +{% block content %} +
+
+
+ {% + with + context = supplier.name, + heading = section.name, + smaller = True + %} + {% include "toolkit/page-heading.html" %} + {% endwith %} +
+
+ + +
+
+
+ {% for question in section.questions %} + {{ forms[question.type](question, declaration, {}) }} + {% endfor %} + {% + with + type = "save", + label = "Save and return to summary" + %} + {% include "toolkit/button.html" %} + {% endwith %} +

+ Return without saving +

+
+
+
+
+
+{% endblock %} diff --git a/app/templates/suppliers/view_declaration.html b/app/templates/suppliers/view_declaration.html new file mode 100644 index 000000000..698a1771e --- /dev/null +++ b/app/templates/suppliers/view_declaration.html @@ -0,0 +1,44 @@ +{% import "toolkit/summary-table.html" as summary %} + +{% extends "_base_page.html" %} + +{% block page_title %} + Change {{ framework.name }} declaration +{% endblock %} + +{% block content %} +
+
+
+ {% + with + context = supplier.name, + heading = framework.name + " declaration", + smaller = True + %} + {% include "toolkit/page-heading.html" %} + {% endwith %} +
+
+ + {% for section in content %} + {{ summary.heading(section.name) }} + {{ summary.top_link("Edit", url_for('.edit_supplier_declaration_section', supplier_id=supplier.id, framework_slug=framework.slug, section_id=section.id)) }} + {% call(item) summary.list_table( + section.questions, + caption="Declaration", + empty_message="This supplier not made a declaration", + field_headings=[ + 'Declaration question', + 'Declaration answer' + ] + ) %} + {% call summary.row() %} + {{ summary.field_name(item.question) }} + {{ summary[item.type](declaration[item.id]) }} + {% endcall %} + {% endcall %} + {% endfor %} +
+ +{% endblock %} diff --git a/app/templates/view_suppliers.html b/app/templates/view_suppliers.html index 2c2a55e0d..fa3098c35 100644 --- a/app/templates/view_suppliers.html +++ b/app/templates/view_suppliers.html @@ -23,20 +23,31 @@ {% endwith %}
- {% call(item) summary.list_table( - suppliers, - caption="Suppliers", - empty_message="No suppliers were found", - field_headings=[ + {% if current_user.has_role("admin") %} + {% set field_headings = [ "Name", - summary.hidden_field_heading("Edit name"), + summary.hidden_field_heading("Change name"), summary.hidden_field_heading("Users"), summary.hidden_field_heading("Services"), - ] if current_user.has_role('admin') else [ + ] %} + {% elif current_user.has_role("admin-ccs-sourcing") %} + {% set field_headings = [ + "Name", + summary.hidden_field_heading("Change declarations"), + ] %} + {% else %} + {% set field_headings = [ "Name", summary.hidden_field_heading("Users"), summary.hidden_field_heading("Services"), - ], + ] %} + {% endif %} + + {% call(item) summary.list_table( + suppliers, + caption="Suppliers", + empty_message="No suppliers were found", + field_headings=field_headings, field_headings_visible=True) %} {% call summary.row() %} @@ -44,8 +55,13 @@ {% if current_user.has_role('admin') %} {{ summary.edit_link("Change name", url_for(".edit_supplier_name", supplier_id=item.id)) }} {% endif %} - {{ summary.edit_link("Users", url_for(".find_supplier_users", supplier_id=item.id)) }} - {{ summary.edit_link("Services", url_for(".find_supplier_services", supplier_id=item.id)) }} + {% if current_user.has_role('admin-ccs-sourcing') %} + {{ summary.edit_link("G-Cloud 7 declaration", url_for(".view_supplier_declaration", supplier_id=item.id, framework_slug="g-cloud-7")) }} + {% endif %} + {% if current_user.has_any_role('admin', 'admin-ccs-category') %} + {{ summary.edit_link("Users", url_for(".find_supplier_users", supplier_id=item.id)) }} + {{ summary.edit_link("Services", url_for(".find_supplier_services", supplier_id=item.id)) }} + {% endif %} {% endcall %} {% endcall %}
From 44d2885d7c1af4e21b03f756343d177cfe88456f Mon Sep 17 00:00:00 2001 From: Rob Young Date: Tue, 13 Oct 2015 11:49:41 +0100 Subject: [PATCH 3/8] Only show relevant search fields to users Only show the search fields that a user can interact with on the index page. For example, admin-ccs-sourcing can only search for suppliers by name so that is the only form they can see. --- app/templates/index.html | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/app/templates/index.html b/app/templates/index.html index b14cb7e3e..7152c65ec 100644 --- a/app/templates/index.html +++ b/app/templates/index.html @@ -35,6 +35,7 @@ {% if current_user.has_any_role('admin', 'admin-ccs-category', 'admin-ccs-sourcing') %}
+ {% if current_user.has_any_role('admin', 'admin-ccs-category') %}

@@ -61,6 +62,7 @@

+ {% endif %}
@@ -71,6 +73,8 @@
+ {% if current_user.has_any_role('admin') %} +

@@ -79,6 +83,8 @@

+ + {% endif %}
{% endif %} From 4619a9629c8f03e68bf6354f00a8c98f58e973fe Mon Sep 17 00:00:00 2001 From: Rob Young Date: Tue, 13 Oct 2015 15:31:12 +0100 Subject: [PATCH 4/8] Reorder load_manifest parameters --- app/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/__init__.py b/app/__init__.py index c16390e78..6b0008888 100644 --- a/app/__init__.py +++ b/app/__init__.py @@ -19,7 +19,7 @@ login_manager = LoginManager() content_loader = ContentLoader('app/content') -content_loader.load_manifest('g-cloud-6', 'edit_service_as_admin', 'services') +content_loader.load_manifest('g-cloud-6', 'services', 'edit_service_as_admin') content_loader.load_manifest('g-cloud-7', 'declaration', 'declaration') From de9fe095dcebce857779a1c9c361064cf760009f Mon Sep 17 00:00:00 2001 From: Rob Young Date: Wed, 14 Oct 2015 09:46:04 +0100 Subject: [PATCH 5/8] Fix failing tests from content loader --- app/main/views/services.py | 2 +- tests/app/main/views/test_services.py | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/app/main/views/services.py b/app/main/views/services.py index 3aba200df..067c57966 100644 --- a/app/main/views/services.py +++ b/app/main/views/services.py @@ -51,7 +51,7 @@ def view(service_id): return redirect(url_for('.index')) service_data['priceString'] = format_service_price(service_data) - content = content_loader.get_builder('g-cloud-6', 'edit_service_as_admin') + content = content_loader.get_builder('g-cloud-6', 'edit_service_as_admin').filter(service_data) template_data = get_template_data( sections=content, diff --git a/tests/app/main/views/test_services.py b/tests/app/main/views/test_services.py index 8a9969166..647730f06 100644 --- a/tests/app/main/views/test_services.py +++ b/tests/app/main/views/test_services.py @@ -454,15 +454,15 @@ def test_cannot_get_archived_services_for_nonexistent_service_ids(self): response = self._get_archived_services_response('30', '40') self.assertEqual(404, response.status_code) - @mock.patch('app.main.views.services.service_content') - def test_can_get_archived_services_with_dates_and_diffs(self, service_content): + @mock.patch('app.main.views.services.content_loader') + def test_can_get_archived_services_with_dates_and_diffs(self, content_loader): class TestBuilder(object): @staticmethod def filter(*args): return self.TestContent() - service_content.get_builder.return_value = TestBuilder() + content_loader.get_builder.return_value = TestBuilder() response = self._get_archived_services_response('10', '20') # check title is there @@ -497,14 +497,14 @@ def filter(*args): self.strip_all_whitespace(response.get_data(as_text=True)) ) - @mock.patch('app.main.views.services.service_content') - def test_can_get_archived_services_with_differing_keys(self, service_content): + @mock.patch('app.main.views.services.content_loader') + def test_can_get_archived_services_with_differing_keys(self, content_loader): class TestBuilder(object): @staticmethod def filter(*args): return self.TestContent() - service_content.get_builder.return_value = TestBuilder() + content_loader.get_builder.return_value = TestBuilder() response = self._get_archived_services_response('10', '50') self.assertEqual(200, response.status_code) From 03764c94337f82582bab4d15cdad6ef04de9bffa Mon Sep 17 00:00:00 2001 From: Rob Young Date: Wed, 14 Oct 2015 10:19:13 +0100 Subject: [PATCH 6/8] Rename my-special-page to declaration-summary It's just not _that_ special. --- app/assets/scss/views/_view_declaration.scss | 2 +- app/templates/suppliers/view_declaration.html | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/assets/scss/views/_view_declaration.scss b/app/assets/scss/views/_view_declaration.scss index 4c0b62704..3ee6b4436 100644 --- a/app/assets/scss/views/_view_declaration.scss +++ b/app/assets/scss/views/_view_declaration.scss @@ -1,3 +1,3 @@ -.my-special-page thead .summary-item-field-heading-first { +.declaration-summary thead .summary-item-field-heading-first { width: 60%; } diff --git a/app/templates/suppliers/view_declaration.html b/app/templates/suppliers/view_declaration.html index 698a1771e..697b76463 100644 --- a/app/templates/suppliers/view_declaration.html +++ b/app/templates/suppliers/view_declaration.html @@ -7,7 +7,7 @@ {% endblock %} {% block content %} -
+
{% From 3cf3bff832a6b63409d46fee5ff7986a5ffd357d Mon Sep 17 00:00:00 2001 From: Rob Young Date: Wed, 14 Oct 2015 12:52:38 +0100 Subject: [PATCH 7/8] Move user loading into LoggedInApplicationTest It should only be present for logged in users. --- tests/app/helpers.py | 20 ++++++++++---------- tests/app/main/views/test_login.py | 18 ++++++++++++------ 2 files changed, 22 insertions(+), 16 deletions(-) diff --git a/tests/app/helpers.py b/tests/app/helpers.py index 95ff61cec..9b9493318 100644 --- a/tests/app/helpers.py +++ b/tests/app/helpers.py @@ -24,19 +24,9 @@ def setUp(self): ) self._default_suffix_patch.start() - self._user_callback = login_manager.user_callback - login_manager.user_loader(self.user_loader) - - def user_loader(self, user_id): - if user_id: - return User( - user_id, 'test@example.com', None, None, False, True, 'tester', 'admin' - ) - def tearDown(self): self._s3_patch.stop() self._default_suffix_patch.stop() - login_manager.user_loader(self._user_callback) def load_example_listing(self, name): file_path = os.path.join("example_responses", "{}.json".format(name)) @@ -89,8 +79,18 @@ def setUp(self): 'password': '1234567890' }) + self._user_callback = login_manager.user_callback + login_manager.user_loader(self.user_loader) + + def user_loader(self, user_id): + if user_id: + return User( + user_id, 'test@example.com', None, None, False, True, 'tester', 'admin' + ) + def tearDown(self): self._data_api_client.stop() + login_manager.user_loader(self._user_callback) def _replace_whitespace(self, string, replacement_substring=""): # Replace all runs of whitespace with replacement_substring diff --git a/tests/app/main/views/test_login.py b/tests/app/main/views/test_login.py index f69806a68..940163bb7 100644 --- a/tests/app/main/views/test_login.py +++ b/tests/app/main/views/test_login.py @@ -36,9 +36,11 @@ def test_should_show_login_page(self): assert_equal(res.status_code, 200) assert_in("Administrator login", res.get_data(as_text=True)) + @mock.patch('app.data_api_client') @mock.patch('app.main.views.login.data_api_client') - def test_valid_login(self, data_api_client): - data_api_client.authenticate_user.return_value = user_data() + def test_valid_login(self, login_data_api_client, init_data_api_client): + login_data_api_client.authenticate_user.return_value = user_data() + init_data_api_client.get_user.return_value = user_data() res = self.client.post('/admin/login', data={ 'email_address': 'valid@email.com', 'password': '1234567890' @@ -101,9 +103,11 @@ def test_should_have_cookie_on_redirect(self, data_api_client): assert_in('HttpOnly', cookie_parts) assert_in('Path=/admin', cookie_parts) + @mock.patch('app.data_api_client') @mock.patch('app.main.views.login.data_api_client') - def test_should_redirect_to_login_on_logout(self, data_api_client): - data_api_client.authenticate_user.return_value = user_data() + def test_should_redirect_to_login_on_logout(self, login_data_api_client, init_data_api_client): + login_data_api_client.authenticate_user.return_value = user_data() + init_data_api_client.get_user.return_value = user_data() self.client.post('/admin/login', data={ 'email_address': 'valid@example.com', 'password': '1234567890', @@ -112,9 +116,11 @@ def test_should_redirect_to_login_on_logout(self, data_api_client): assert_equal(res.status_code, 302) assert_equal(urlsplit(res.location).path, '/admin/login') + @mock.patch('app.data_api_client') @mock.patch('app.main.views.login.data_api_client') - def test_logout_should_log_user_out(self, data_api_client): - data_api_client.authenticate_user.return_value = user_data() + def test_logout_should_log_user_out(self, login_data_api_client, init_data_api_client): + login_data_api_client.authenticate_user.return_value = user_data() + init_data_api_client.get_user.return_value = user_data() self.client.post('/admin/login', data={ 'email_address': 'valid@example.com', 'password': '1234567890', From e9a8bf8af31a44549f4c8ec6cbfe9a9f98ea256e Mon Sep 17 00:00:00 2001 From: Rob Young Date: Wed, 14 Oct 2015 13:53:34 +0100 Subject: [PATCH 8/8] Add tests for supplier declaration --- app/main/views/suppliers.py | 28 +++- example_responses/declaration_response.json | 17 +++ example_responses/framework_response.json | 6 + tests/app/helpers.py | 5 +- tests/app/main/views/test_suppliers.py | 137 +++++++++++++++++++- 5 files changed, 187 insertions(+), 6 deletions(-) create mode 100644 example_responses/declaration_response.json create mode 100644 example_responses/framework_response.json diff --git a/app/main/views/suppliers.py b/app/main/views/suppliers.py index 20176b950..acee1a960 100644 --- a/app/main/views/suppliers.py +++ b/app/main/views/suppliers.py @@ -44,7 +44,12 @@ def edit_supplier_name(supplier_id): def view_supplier_declaration(supplier_id, framework_slug): supplier = data_api_client.get_supplier(supplier_id)['suppliers'] framework = data_api_client.get_framework(framework_slug)['frameworks'] - declaration = data_api_client.get_supplier_declaration(supplier_id, framework_slug)['declaration'] + try: + declaration = data_api_client.get_supplier_declaration(supplier_id, framework_slug)['declaration'] + except APIError as e: + if e.status_code != 404: + raise + declaration = {} content = content_loader.get_builder(framework_slug, 'declaration').filter(declaration) @@ -65,16 +70,24 @@ def view_supplier_declaration(supplier_id, framework_slug): def edit_supplier_declaration_section(supplier_id, framework_slug, section_id): supplier = data_api_client.get_supplier(supplier_id)['suppliers'] framework = data_api_client.get_framework(framework_slug)['frameworks'] - declaration = data_api_client.get_supplier_declaration(supplier_id, framework_slug)['declaration'] + try: + declaration = data_api_client.get_supplier_declaration(supplier_id, framework_slug)['declaration'] + except APIError as e: + if e.status_code != 404: + raise + declaration = {} content = content_loader.get_builder(framework_slug, 'declaration').filter(declaration) + section = content.get_section(section_id) + if section is None: + abort(404) return render_template( "suppliers/edit_declaration.html", supplier=supplier, framework=framework, declaration=declaration, - section=content.get_section(section_id), + section=section, **get_template_data()) @@ -84,10 +97,17 @@ def edit_supplier_declaration_section(supplier_id, framework_slug, section_id): def update_supplier_declaration_section(supplier_id, framework_slug, section_id): supplier = data_api_client.get_supplier(supplier_id)['suppliers'] framework = data_api_client.get_framework(framework_slug)['frameworks'] - declaration = data_api_client.get_supplier_declaration(supplier_id, framework_slug)['declaration'] + try: + declaration = data_api_client.get_supplier_declaration(supplier_id, framework_slug)['declaration'] + except APIError as e: + if e.status_code != 404: + raise + declaration = {} content = content_loader.get_builder(framework_slug, 'declaration').filter(declaration) section = content.get_section(section_id) + if section is None: + abort(404) posted_data = section.get_data(request.form) diff --git a/example_responses/declaration_response.json b/example_responses/declaration_response.json new file mode 100644 index 000000000..cb2ea1ef8 --- /dev/null +++ b/example_responses/declaration_response.json @@ -0,0 +1,17 @@ +{ + "declaration": { + "PR1": true, + "PR2": false, + "SQC2": true, + "PR5": false, + "PR3": true, + "PR4": false, + "SQ1-3": [], + "SQA2": false, + "SQA3": true, + "SQA4": false, + "SQA5": true, + "AQA3": false, + "SQC3": ["race", "sexual orientation"] + } +} diff --git a/example_responses/framework_response.json b/example_responses/framework_response.json new file mode 100644 index 000000000..70fe109c2 --- /dev/null +++ b/example_responses/framework_response.json @@ -0,0 +1,6 @@ +{ + "frameworks": { + "slug": "g-cloud-7", + "name": "G-Cloud 7" + } +} diff --git a/tests/app/helpers.py b/tests/app/helpers.py index 9b9493318..948ffb890 100644 --- a/tests/app/helpers.py +++ b/tests/app/helpers.py @@ -54,8 +54,11 @@ def status_code(self): class LoggedInApplicationTest(BaseApplicationTest): + user_role = 'admin' + def setUp(self): super(LoggedInApplicationTest, self).setUp() + patch_config = { 'authenticate_user.return_value': { 'users': { @@ -85,7 +88,7 @@ def setUp(self): def user_loader(self, user_id): if user_id: return User( - user_id, 'test@example.com', None, None, False, True, 'tester', 'admin' + user_id, 'test@example.com', None, None, False, True, 'tester', self.user_role ) def tearDown(self): diff --git a/tests/app/main/views/test_suppliers.py b/tests/app/main/views/test_suppliers.py index a414f9a6d..262cc92ed 100644 --- a/tests/app/main/views/test_suppliers.py +++ b/tests/app/main/views/test_suppliers.py @@ -6,8 +6,9 @@ from io import BytesIO as StringIO import mock from lxml import html +from nose.tools import eq_ from nose.tools import assert_equals -from dmutils.apiclient.errors import HTTPError +from dmutils.apiclient.errors import HTTPError, APIError from dmutils.email import MandrillException from dmutils.audit import AuditTypes from ...helpers import LoggedInApplicationTest, Response @@ -615,3 +616,137 @@ def test_should_be_a_503_if_email_fails(self, data_api_client, send_email): ) self.assertEqual(res.status_code, 503) + + +@mock.patch('app.main.views.suppliers.data_api_client') +class TestViewingASupplierDeclaration(LoggedInApplicationTest): + user_role = 'admin-ccs-sourcing' + + def test_should_not_be_visible_to_admin_users(self, data_api_client): + self.user_role = 'admin' + + response = self.client.get('/admin/suppliers/1234/edit/declarations/g-cloud-7') + + eq_(response.status_code, 403) + + def test_should_404_if_supplier_does_not_exist(self, data_api_client): + data_api_client.get_supplier.side_effect = APIError(Response(404)) + + response = self.client.get('/admin/suppliers/1234/edit/declarations/g-cloud-7') + + eq_(response.status_code, 404) + data_api_client.get_supplier.assert_called_with('1234') + assert not data_api_client.get_framework.called + + def test_should_404_if_framework_does_not_exist(self, data_api_client): + data_api_client.get_supplier.return_value = self.load_example_listing('supplier_response') + data_api_client.get_framework.side_effect = APIError(Response(404)) + + response = self.client.get('/admin/suppliers/1234/edit/declarations/g-cloud-7') + + eq_(response.status_code, 404) + data_api_client.get_supplier.assert_called_with('1234') + data_api_client.get_framework.assert_called_with('g-cloud-7') + + def test_should_not_404_if_declaration_does_not_exist(self, data_api_client): + data_api_client.get_supplier.return_value = self.load_example_listing('supplier_response') + data_api_client.get_framework.return_value = self.load_example_listing('framework_response') + data_api_client.get_supplier_declaration.side_effect = APIError(Response(404)) + + response = self.client.get('/admin/suppliers/1234/edit/declarations/g-cloud-7') + + eq_(response.status_code, 200) + data_api_client.get_supplier.assert_called_with('1234') + data_api_client.get_framework.assert_called_with('g-cloud-7') + data_api_client.get_supplier_declaration.assert_called_with('1234', 'g-cloud-7') + + def test_should_show_declaration(self, data_api_client): + data_api_client.get_supplier.return_value = self.load_example_listing('supplier_response') + data_api_client.get_framework.return_value = self.load_example_listing('framework_response') + data_api_client.get_supplier_declaration.return_value = self.load_example_listing('declaration_response') + + response = self.client.get('/admin/suppliers/1234/edit/declarations/g-cloud-7') + document = html.fromstring(response.get_data(as_text=True)) + + eq_(response.status_code, 200) + data = document.cssselect('.summary-item-row td.summary-item-field') + eq_(data[0].text_content().strip(), "Yes") + + +@mock.patch('app.main.views.suppliers.data_api_client') +class TestEditingASupplierDeclaration(LoggedInApplicationTest): + user_role = 'admin-ccs-sourcing' + + def test_should_not_be_visible_to_admin_users(self, data_api_client): + self.user_role = 'admin' + + response = self.client.get('/admin/suppliers/1234/edit/declarations/g-cloud-7/section') + + eq_(response.status_code, 403) + + def test_should_404_if_supplier_does_not_exist(self, data_api_client): + data_api_client.get_supplier.side_effect = APIError(Response(404)) + + response = self.client.get('/admin/suppliers/1234/edit/declarations/g-cloud-7/g_cloud_7_essentials') + + eq_(response.status_code, 404) + data_api_client.get_supplier.assert_called_with('1234') + assert not data_api_client.get_framework.called + + def test_should_404_if_framework_does_not_exist(self, data_api_client): + data_api_client.get_supplier.return_value = self.load_example_listing('supplier_response') + data_api_client.get_framework.side_effect = APIError(Response(404)) + + response = self.client.get('/admin/suppliers/1234/edit/declarations/g-cloud-7/g_cloud_7_essentials') + + eq_(response.status_code, 404) + data_api_client.get_supplier.assert_called_with('1234') + data_api_client.get_framework.assert_called_with('g-cloud-7') + + def test_should_404_if_section_does_not_exist(self, data_api_client): + data_api_client.get_supplier.return_value = self.load_example_listing('supplier_response') + data_api_client.get_framework.return_value = self.load_example_listing('framework_response') + data_api_client.get_supplier_declaration.side_effect = APIError(Response(404)) + + response = self.client.get('/admin/suppliers/1234/edit/declarations/g-cloud-7/not_a_section') + + eq_(response.status_code, 404) + + def test_should_not_404_if_declaration_does_not_exist(self, data_api_client): + data_api_client.get_supplier.return_value = self.load_example_listing('supplier_response') + data_api_client.get_framework.return_value = self.load_example_listing('framework_response') + data_api_client.get_supplier_declaration.side_effect = APIError(Response(404)) + + response = self.client.get('/admin/suppliers/1234/edit/declarations/g-cloud-7/g_cloud_7_essentials') + + eq_(response.status_code, 200) + data_api_client.get_supplier.assert_called_with('1234') + data_api_client.get_framework.assert_called_with('g-cloud-7') + data_api_client.get_supplier_declaration.assert_called_with('1234', 'g-cloud-7') + + def test_should_prefill_form_with_declaration(self, data_api_client): + data_api_client.get_supplier.return_value = self.load_example_listing('supplier_response') + data_api_client.get_framework.return_value = self.load_example_listing('framework_response') + data_api_client.get_supplier_declaration.return_value = self.load_example_listing('declaration_response') + + response = self.client.get('/admin/suppliers/1234/edit/declarations/g-cloud-7/g_cloud_7_essentials') + document = html.fromstring(response.get_data(as_text=True)) + + eq_(response.status_code, 200) + assert document.cssselect('#input-PR1-yes')[0].checked + assert not document.cssselect('#input-PR1-no')[0].checked + + def test_should_set_declaration(self, data_api_client): + data_api_client.get_supplier.return_value = self.load_example_listing('supplier_response') + data_api_client.get_framework.return_value = self.load_example_listing('framework_response') + data_api_client.get_supplier_declaration.return_value = self.load_example_listing('declaration_response') + + response = self.client.post( + '/admin/suppliers/1234/edit/declarations/g-cloud-7/g_cloud_7_essentials', + data={'PR1': 'false'}) + + declaration = self.load_example_listing('declaration_response')['declaration'] + declaration['PR1'] = False + + data_api_client.set_supplier_declaration.assert_called_with( + '1234', 'g-cloud-7', declaration, 'test@example.com')