From e1fd63e184719866fe6304c8b74b8c563c83e242 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Wed, 25 Apr 2018 10:24:32 +0100 Subject: [PATCH] Rewrite navigation as a class Because we have multiple navigations, which will share the same methods (by subclassing) but different mappings of navigation items to endpoints by overriding the `.mapping` and `.exclude` attributes. --- app/__init__.py | 7 +- app/navigation.py | 470 ++++++++++++++++++----------------- app/templates/main_nav.html | 12 +- tests/app/test_navigation.py | 20 +- 4 files changed, 263 insertions(+), 246 deletions(-) diff --git a/app/__init__.py b/app/__init__.py index 9eb681237..0eb444f53 100644 --- a/app/__init__.py +++ b/app/__init__.py @@ -36,9 +36,10 @@ from notifications_utils.formatters import formatted_list from werkzeug.exceptions import abort from werkzeug.local import LocalProxy -from app import proxy_fix, navigation +from app import proxy_fix from app.config import configs from app.asset_fingerprinter import AssetFingerprinter +from app.navigation import MainNavigation from app.notify_client.service_api_client import ServiceAPIClient from app.notify_client.api_key_api_client import ApiKeyApiClient from app.notify_client.invite_api_client import InviteApiClient @@ -89,6 +90,8 @@ current_service = LocalProxy(partial(_lookup_req_object, 'service')) # The current organisation attached to the request stack. current_organisation = LocalProxy(partial(_lookup_req_object, 'organisation')) +main_navigation = MainNavigation() + def create_app(application): setup_commands(application) @@ -173,7 +176,7 @@ def init_app(application): @application.context_processor def _nav_selected(): - return {'nav_selected': navigation.nav_selected} + return {'main_navigation': main_navigation} @application.before_request def record_start_time(): diff --git a/app/navigation.py b/app/navigation.py index 46054eec7..78bd6b102 100644 --- a/app/navigation.py +++ b/app/navigation.py @@ -2,237 +2,251 @@ from itertools import chain from flask import request -mapping = { - 'dashboard': { - 'conversation', - 'inbox', - 'monthly', - 'service_dashboard', - 'template_usage', - 'view_job', - 'view_jobs', - 'view_notification', - 'view_notifications', - }, - 'templates': { - 'action_blocked', - 'add_service_template', - 'add_template_by_type', - 'check_messages', - 'check_notification', - 'choose_template', - 'confirm_redact_template', - 'conversation_reply', - 'delete_service_template', - 'edit_service_template', - 'send_messages', - 'send_one_off', - 'send_one_off_step', - 'send_test', - 'send_test_preview', - 'send_test_step', - 'set_sender', - 'set_template_sender', - 'view_template', - 'view_template_version', - 'view_template_versions', - }, - 'team-members': { - 'edit_user_permissions', - 'invite_user', - 'manage_users', - 'remove_user_from_service', - }, - 'usage': { - 'usage', - }, - 'settings': { - 'link_service_to_organisation', - 'request_to_go_live', - 'service_add_email_reply_to', - 'service_add_letter_contact', - 'service_add_sms_sender', - 'service_edit_email_reply_to', - 'service_edit_letter_contact', - 'service_edit_sms_sender', - 'service_email_reply_to', - 'service_letter_contact_details', - 'service_name_change', - 'service_name_change_confirm', - 'service_set_auth_type', - 'service_set_email', - 'service_set_email_branding', - 'service_set_inbound_number', - 'service_set_inbound_sms', - 'service_set_international_sms', - 'service_set_letter_contact_block', - 'service_set_letters', - 'service_set_reply_to_email', - 'service_set_sms', - 'service_set_sms_prefix', - 'service_settings', - 'service_sms_senders', - 'set_free_sms_allowance', - 'set_letter_branding', - 'set_organisation_type', - 'submit_request_to_go_live', - }, - 'api-integration': { - 'api_callbacks', - 'api_documentation', - 'api_integration', - 'api_keys', - 'create_api_key', - 'delivery_status_callback', - 'received_text_messages_callback', - 'revoke_api_key', - 'whitelist', - }, -} -exclude = { - 'accept_invite', - 'accept_org_invite', - 'add_organisation', - 'add_service', - 'agreement', - 'archive_service', - 'bat_phone', - 'callbacks', - 'cancel_invited_org_user', - 'cancel_invited_user', - 'cancel_job', - 'check_and_resend_text_code', - 'check_and_resend_verification_code', - 'check_messages_preview', - 'choose_account', - 'choose_service', - 'confirm_edit_organisation_name', - 'conversation_reply_with_template', - 'conversation_updates', - 'cookies', - 'create_email_branding', - 'delivery_and_failure', - 'design_content', - 'documentation', - 'download_agreement', - 'download_notifications_csv', - 'edit_organisation_name', - 'edit_provider', - 'edit_user_org_permissions', - 'email_branding', - 'email_not_received', - 'email_template', - 'error', - 'features', - 'feedback', - 'forgot_password', - 'get_example_csv', - 'get_notifications_as_json', - 'go_to_dashboard_after_tour', - 'inbound_sms_admin', - 'inbox_download', - 'inbox_updates', - 'index', - 'information_risk_management', - 'information_security', - 'integration_testing', - 'invite_org_user', - 'letter_jobs', - 'live_services', - 'manage_org_users', - 'new_password', - 'old_feedback', - 'old_integration_testing', - 'old_roadmap', - 'old_service_dashboard', - 'old_submit_feedback', - 'old_terms', - 'old_using_notify', - 'organisation_dashboard', - 'organisation_settings', - 'organisations', - 'platform_admin', - 'pricing', - 'redact_template', - 'register', - 'register_from_invite', - 'register_from_org_invite', - 'registration_continue', - 'remove_user_from_organisation', - 'resend_email_link', - 'resend_email_verification', - 'resume_service', - 'roadmap', - 'security', - 'send_notification', - 'service_dashboard_updates', - 'service_switch_can_send_email', - 'service_switch_can_send_precompiled_letter', - 'service_switch_can_send_sms', - 'service_switch_email_auth', - 'service_switch_live', - 'service_switch_research_mode', - 'services_or_dashboard', - 'show_accounts_or_dashboard', - 'sign_in', - 'sign_out', - 'start_job', - 'start_tour', - 'styleguide', - 'support', - 'suspend_service', - 'temp_service_history', - 'template_history', - 'terms', - 'thanks', - 'triage', - 'trial_mode', - 'trial_services', - 'two_factor', - 'two_factor_email', - 'two_factor_email_sent', - 'update_email_branding', - 'user_profile', - 'user_profile_email', - 'user_profile_email_authenticate', - 'user_profile_email_confirm', - 'user_profile_mobile_number', - 'user_profile_mobile_number_authenticate', - 'user_profile_mobile_number_confirm', - 'user_profile_name', - 'user_profile_password', - 'using_notify', - 'verify', - 'verify_email', - 'verify_mobile', - 'view_job_csv', - 'view_job_updates', - 'view_letter_notification_as_preview', - 'view_letter_template_preview', - 'view_notification_updates', - 'view_notifications_csv', - 'view_provider', - 'view_providers', - 'view_template_version_preview', -} +class Navigation: -mapping = { - navigation: { - 'main.{}'.format(endpoint) for endpoint in endpoints - } for navigation, endpoints in mapping.items() -} + mapping = {} + exclude = {} -endpoints_with_navigation = tuple(chain.from_iterable(( - endpoints for navigation_item, endpoints in mapping.items() -))) + def __init__(self): + self.mapping = { + navigation: { + 'main.{}'.format(endpoint) for endpoint in endpoints + } for navigation, endpoints in self.mapping.items() + } -endpoints_without_navigation = tuple( - 'main.{}'.format(endpoint) for endpoint in exclude -) + ('static', 'status.show_status') + @property + def endpoints_with_navigation(self): + return tuple(chain.from_iterable(( + endpoints + for navigation_item, endpoints in self.mapping.items() + ))) + + @property + def endpoints_without_navigation(self): + return tuple( + 'main.{}'.format(endpoint) for endpoint in self.exclude + ) + ('static', 'status.show_status') + + def is_selected(self, navigation_item): + if request.endpoint in self.mapping[navigation_item]: + return "class=selected" + return '' -def nav_selected(navigation_item): - if request.endpoint in mapping[navigation_item]: - return "class=selected" - return '' +class MainNavigation(Navigation): + + mapping = { + 'dashboard': { + 'conversation', + 'inbox', + 'monthly', + 'service_dashboard', + 'template_usage', + 'view_job', + 'view_jobs', + 'view_notification', + 'view_notifications', + }, + 'templates': { + 'action_blocked', + 'add_service_template', + 'add_template_by_type', + 'check_messages', + 'check_notification', + 'choose_template', + 'confirm_redact_template', + 'conversation_reply', + 'delete_service_template', + 'edit_service_template', + 'send_messages', + 'send_one_off', + 'send_one_off_step', + 'send_test', + 'send_test_preview', + 'send_test_step', + 'set_sender', + 'set_template_sender', + 'view_template', + 'view_template_version', + 'view_template_versions', + }, + 'team-members': { + 'edit_user_permissions', + 'invite_user', + 'manage_users', + 'remove_user_from_service', + }, + 'usage': { + 'usage', + }, + 'settings': { + 'link_service_to_organisation', + 'request_to_go_live', + 'service_add_email_reply_to', + 'service_add_letter_contact', + 'service_add_sms_sender', + 'service_edit_email_reply_to', + 'service_edit_letter_contact', + 'service_edit_sms_sender', + 'service_email_reply_to', + 'service_letter_contact_details', + 'service_name_change', + 'service_name_change_confirm', + 'service_set_auth_type', + 'service_set_email', + 'service_set_email_branding', + 'service_set_inbound_number', + 'service_set_inbound_sms', + 'service_set_international_sms', + 'service_set_letter_contact_block', + 'service_set_letters', + 'service_set_reply_to_email', + 'service_set_sms', + 'service_set_sms_prefix', + 'service_settings', + 'service_sms_senders', + 'set_free_sms_allowance', + 'set_letter_branding', + 'set_organisation_type', + 'submit_request_to_go_live', + }, + 'api-integration': { + 'api_callbacks', + 'api_documentation', + 'api_integration', + 'api_keys', + 'create_api_key', + 'delivery_status_callback', + 'received_text_messages_callback', + 'revoke_api_key', + 'whitelist', + }, + } + + exclude = { + 'accept_invite', + 'accept_org_invite', + 'add_organisation', + 'add_service', + 'agreement', + 'archive_service', + 'bat_phone', + 'callbacks', + 'cancel_invited_org_user', + 'cancel_invited_user', + 'cancel_job', + 'check_and_resend_text_code', + 'check_and_resend_verification_code', + 'check_messages_preview', + 'choose_account', + 'choose_service', + 'confirm_edit_organisation_name', + 'conversation_reply_with_template', + 'conversation_updates', + 'cookies', + 'create_email_branding', + 'delivery_and_failure', + 'design_content', + 'documentation', + 'download_agreement', + 'download_notifications_csv', + 'edit_organisation_name', + 'edit_provider', + 'edit_user_org_permissions', + 'email_branding', + 'email_not_received', + 'email_template', + 'error', + 'features', + 'feedback', + 'forgot_password', + 'get_example_csv', + 'get_notifications_as_json', + 'go_to_dashboard_after_tour', + 'inbound_sms_admin', + 'inbox_download', + 'inbox_updates', + 'index', + 'information_risk_management', + 'information_security', + 'integration_testing', + 'invite_org_user', + 'letter_jobs', + 'live_services', + 'manage_org_users', + 'new_password', + 'old_feedback', + 'old_integration_testing', + 'old_roadmap', + 'old_service_dashboard', + 'old_submit_feedback', + 'old_terms', + 'old_using_notify', + 'organisation_dashboard', + 'organisation_settings', + 'organisations', + 'platform_admin', + 'pricing', + 'redact_template', + 'register', + 'register_from_invite', + 'register_from_org_invite', + 'registration_continue', + 'remove_user_from_organisation', + 'resend_email_link', + 'resend_email_verification', + 'resume_service', + 'roadmap', + 'security', + 'send_notification', + 'service_dashboard_updates', + 'service_switch_can_send_email', + 'service_switch_can_send_precompiled_letter', + 'service_switch_can_send_sms', + 'service_switch_email_auth', + 'service_switch_live', + 'service_switch_research_mode', + 'services_or_dashboard', + 'show_accounts_or_dashboard', + 'sign_in', + 'sign_out', + 'start_job', + 'start_tour', + 'styleguide', + 'support', + 'suspend_service', + 'temp_service_history', + 'template_history', + 'terms', + 'thanks', + 'triage', + 'trial_mode', + 'trial_services', + 'two_factor', + 'two_factor_email', + 'two_factor_email_sent', + 'update_email_branding', + 'user_profile', + 'user_profile_email', + 'user_profile_email_authenticate', + 'user_profile_email_confirm', + 'user_profile_mobile_number', + 'user_profile_mobile_number_authenticate', + 'user_profile_mobile_number_confirm', + 'user_profile_name', + 'user_profile_password', + 'using_notify', + 'verify', + 'verify_email', + 'verify_mobile', + 'view_job_csv', + 'view_job_updates', + 'view_letter_notification_as_preview', + 'view_letter_template_preview', + 'view_notification_updates', + 'view_notifications_csv', + 'view_provider', + 'view_providers', + 'view_template_version_preview', + } diff --git a/app/templates/main_nav.html b/app/templates/main_nav.html index 91b1883a1..6be863235 100644 --- a/app/templates/main_nav.html +++ b/app/templates/main_nav.html @@ -43,19 +43,19 @@ {% else %} diff --git a/tests/app/test_navigation.py b/tests/app/test_navigation.py index 392593f61..acbfcb6de 100644 --- a/tests/app/test_navigation.py +++ b/tests/app/test_navigation.py @@ -1,7 +1,7 @@ import pytest from tests.conftest import SERVICE_ONE_ID, app_ -from app import navigation +from app.navigation import MainNavigation all_endpoints = [ rule.endpoint for rule in next(app_(None)).url_map.iter_rules() @@ -9,30 +9,30 @@ all_endpoints = [ def test_navigation_items_are_properly_defined(): - for endpoint in navigation.endpoints_with_navigation: + for endpoint in MainNavigation().endpoints_with_navigation: assert endpoint in all_endpoints - assert endpoint not in navigation.endpoints_without_navigation - assert navigation.endpoints_with_navigation.count(endpoint) == 1 + assert endpoint not in MainNavigation().endpoints_without_navigation + assert MainNavigation().endpoints_with_navigation.count(endpoint) == 1 def test_excluded_navigation_items_are_properly_defined(): - for endpoint in navigation.endpoints_without_navigation: + for endpoint in MainNavigation().endpoints_without_navigation: assert endpoint in all_endpoints - assert endpoint not in navigation.endpoints_with_navigation - assert navigation.endpoints_without_navigation.count(endpoint) == 1 + assert endpoint not in MainNavigation().endpoints_with_navigation + assert MainNavigation().endpoints_without_navigation.count(endpoint) == 1 def test_all_endpoints_are_covered(): for endpoint in all_endpoints: assert endpoint in ( - navigation.endpoints_with_navigation + - navigation.endpoints_without_navigation + MainNavigation().endpoints_with_navigation + + MainNavigation().endpoints_without_navigation ) @pytest.mark.xfail(raises=KeyError) def test_raises_on_invalid_navigation_item(client_request): - navigation.nav_selected('foo') + MainNavigation().is_selected('foo') @pytest.mark.parametrize('endpoint, selected_nav_item', [