diff --git a/app/main/views/add_service.py b/app/main/views/add_service.py index 3f8a5e064..a9731c32b 100644 --- a/app/main/views/add_service.py +++ b/app/main/views/add_service.py @@ -41,7 +41,6 @@ def _add_invited_user_to_service(invited_user): def _create_service(service_name, email_from): service_id = service_api_client.create_service(service_name=service_name, - active=False, message_limit=current_app.config['DEFAULT_SERVICE_LIMIT'], restricted=True, user_id=session['user_id'], @@ -69,7 +68,7 @@ def add_service(): service_name = form.name.data service_id = _create_service(service_name, email_from) - if (len(service_api_client.get_services({'user_id': session['user_id']}).get('data', [])) > 1): + if (len(service_api_client.get_active_services({'user_id': session['user_id']}).get('data', [])) > 1): return redirect(url_for('main.service_dashboard', service_id=service_id)) example_sms_template = service_api_client.create_service_template( diff --git a/app/main/views/choose_service.py b/app/main/views/choose_service.py index d5cd2e6b9..d1a58f68d 100644 --- a/app/main/views/choose_service.py +++ b/app/main/views/choose_service.py @@ -12,7 +12,7 @@ def choose_service(): return render_template( 'views/choose-service.html', services=[ServicesBrowsableItem(x) for x in - service_api_client.get_services({'user_id': current_user.id})['data']], + service_api_client.get_active_services({'user_id': current_user.id})['data']], can_add_service=is_gov_user(current_user.email_address) ) @@ -23,7 +23,7 @@ def show_all_services_or_dashboard(): if not current_user.is_authenticated: return redirect(url_for('.index')) - services = service_api_client.get_services({'user_id': current_user.id})['data'] + services = service_api_client.get_active_services({'user_id': current_user.id})['data'] if 1 == len(services): return redirect(url_for('.service_dashboard', service_id=services[0]['id'])) diff --git a/app/main/views/platform_admin.py b/app/main/views/platform_admin.py index 3d613f458..a5485b988 100644 --- a/app/main/views/platform_admin.py +++ b/app/main/views/platform_admin.py @@ -13,6 +13,7 @@ from app.statistics_utils import get_formatted_percentage @login_required @user_has_permissions(admin_override=True) def platform_admin(): + # specifically DO get inactive services services = service_api_client.get_services({'detailed': True})['data'] return render_template( 'views/platform-admin.html', diff --git a/app/main/views/sign_in.py b/app/main/views/sign_in.py index c93dfd847..1ad7a2521 100644 --- a/app/main/views/sign_in.py +++ b/app/main/views/sign_in.py @@ -55,7 +55,7 @@ def sign_in(): user.is_active: confirm_login() - services = service_api_client.get_services({'user_id': str(user.id)}).get('data', []) + services = service_api_client.get_active_services({'user_id': str(user.id)}).get('data', []) if (len(services) == 1): return redirect(url_for('main.service_dashboard', service_id=services[0]['id'])) else: diff --git a/app/main/views/two_factor.py b/app/main/views/two_factor.py index c4d609d64..770d2cf4b 100644 --- a/app/main/views/two_factor.py +++ b/app/main/views/two_factor.py @@ -26,7 +26,7 @@ def two_factor(): if form.validate_on_submit(): try: user = user_api_client.get_user(user_id) - services = service_api_client.get_services({'user_id': str(user_id)}).get('data', []) + services = service_api_client.get_active_services({'user_id': str(user_id)}).get('data', []) # Check if coming from new password page if 'password' in session['user_details']: user.set_password(session['user_details']['password']) diff --git a/app/notify_client/service_api_client.py b/app/notify_client/service_api_client.py index 9766d876c..379bbcb14 100644 --- a/app/notify_client/service_api_client.py +++ b/app/notify_client/service_api_client.py @@ -16,13 +16,13 @@ class ServiceAPIClient(BaseAPIClient): self.service_id = application.config['ADMIN_CLIENT_USER_NAME'] self.api_key = application.config['ADMIN_CLIENT_SECRET'] - def create_service(self, service_name, active, message_limit, restricted, user_id, email_from): + def create_service(self, service_name, message_limit, restricted, user_id, email_from): """ Create a service and return the json. """ data = { "name": service_name, - "active": active, + "active": True, "message_limit": message_limit, "user_id": user_id, "restricted": restricted, @@ -57,11 +57,18 @@ class ServiceAPIClient(BaseAPIClient): '/service/{0}'.format(service_id), params=params) - def get_services(self, *params): + def get_services(self, params_dict=None): """ Retrieve a list of services. """ - return self.get('/service', *params) + return self.get('/service', params=params_dict) + + def get_active_services(self, params_dict=None): + """ + Retrieve a list of active services. + """ + params_dict['only_active'] = True + return self.get_services(params_dict) def update_service( self, diff --git a/docker/Dockerfile-build b/docker/Dockerfile-build index 49a36e251..2ce5a28bd 100644 --- a/docker/Dockerfile-build +++ b/docker/Dockerfile-build @@ -18,6 +18,9 @@ RUN \ zlib1g-dev \ zip \ rlwrap \ + python-dev \ + libffi-dev \ + libssl-dev \ && echo "Install nodejs" \ && cd /tmp \ diff --git a/setup.cfg b/setup.cfg index af94d5aee..95eee0b08 100644 --- a/setup.cfg +++ b/setup.cfg @@ -1,6 +1,6 @@ [pep8] max-line-length = 120 -exclude = ./migrations,./venv,./venv3,./node_modules,./bower_components +exclude = ./migrations,./venv,./venv3,./node_modules,./bower_components,./cache [tool:pytest] norecursedirs = node_modules bower_components diff --git a/tests/app/main/views/test_add_service.py b/tests/app/main/views/test_add_service.py index a1639663b..537ccad58 100644 --- a/tests/app/main/views/test_add_service.py +++ b/tests/app/main/views/test_add_service.py @@ -40,7 +40,6 @@ def test_should_add_service_and_redirect_to_tour_when_no_services(app_, assert mock_get_services_with_no_services.called mock_create_service.assert_called_once_with( service_name='testing the post', - active=False, message_limit=app_.config['DEFAULT_SERVICE_LIMIT'], restricted=True, user_id=api_user_active.id, @@ -73,7 +72,6 @@ def test_should_add_service_and_redirect_to_dashboard_when_existing_service(app_ assert mock_get_services.called mock_create_service.assert_called_once_with( service_name='testing the post', - active=False, message_limit=app_.config['DEFAULT_SERVICE_LIMIT'], restricted=True, user_id=api_user_active.id, diff --git a/tests/app/main/views/test_choose_services.py b/tests/app/main/views/test_choose_services.py index d4399490a..2ca0dbb28 100644 --- a/tests/app/main/views/test_choose_services.py +++ b/tests/app/main/views/test_choose_services.py @@ -1,4 +1,4 @@ -from flask import url_for, session +from flask import url_for def test_should_show_choose_services_page(app_, @@ -18,13 +18,26 @@ def test_should_show_choose_services_page(app_, assert mock_get_services.called assert services['data'][0]['name'] in resp_data assert services['data'][1]['name'] in resp_data - assert 'List all services' not in resp_data + + +def test_should_show_choose_services_page_if_no_services( + client, + mock_login, + api_user_active, +): + # if users last service has been archived there'll be no services + # mock_login already patches get_services to return no data + client.login(api_user_active) + response = client.get(url_for('main.choose_service')) + assert response.status_code == 200 + resp_data = response.get_data(as_text=True) + assert 'Choose service' in resp_data + assert 'Add a new service' in resp_data def test_redirect_if_only_one_service( app_, mock_login, - mock_get_user, api_user_active, mock_get_services_with_one_service ): @@ -41,9 +54,7 @@ def test_redirect_if_only_one_service( def test_redirect_if_multiple_services( app_, mock_login, - mock_get_user, api_user_active, - mock_get_services ): with app_.test_request_context(): with app_.test_client() as client: @@ -57,7 +68,6 @@ def test_redirect_if_multiple_services( def test_redirect_if_service_in_session( app_, mock_login, - mock_get_user, api_user_active, mock_get_services, mock_get_service diff --git a/tests/app/main/views/test_service_settings.py b/tests/app/main/views/test_service_settings.py index ae0484b28..4090ec366 100644 --- a/tests/app/main/views/test_service_settings.py +++ b/tests/app/main/views/test_service_settings.py @@ -545,11 +545,7 @@ def test_switch_service_from_research_mode_to_normal( with app_.test_request_context(): with app_.test_client() as client: service = service_json( - "1234", - "Test Service", - [active_user_with_permissions.id], - message_limit=1000, - active=False, + users=[active_user_with_permissions.id], restricted=True, research_mode=True ) diff --git a/tests/app/main/views/test_sign_in.py b/tests/app/main/views/test_sign_in.py index f57eac9ee..418098eed 100644 --- a/tests/app/main/views/test_sign_in.py +++ b/tests/app/main/views/test_sign_in.py @@ -12,15 +12,9 @@ def test_render_sign_in_returns_sign_in_template(app_): assert 'Forgot your password?' in response.get_data(as_text=True) -def test_logged_in_user_redirects_to_choose_service(app_, - api_user_active, - mock_get_user): - - with app_.test_request_context(): - with app_.test_client() as client: - client.login(api_user_active) - response = client.get(url_for('main.sign_in')) - assert response.location == url_for('main.choose_service', _external=True) +def test_logged_in_user_redirects_to_choose_service(logged_in_client): + response = logged_in_client.get(url_for('main.sign_in')) + assert response.location == url_for('main.choose_service', _external=True) def test_process_sign_in_return_2fa_template(app_, @@ -88,24 +82,44 @@ def test_should_attempt_redirect_when_user_is_pending(app_, assert response.status_code == 302 -def test_not_fresh_session_login(app_, - api_user_active, - mock_login, - mock_get_user_by_email, - mock_verify_password, - mock_get_services_with_one_service): - with app_.test_request_context(): - with app_.test_client() as client: - client.login(api_user_active) - with client.session_transaction() as session: - assert session['_fresh'] - session['_fresh'] = False - # This should skip the two factor - response = client.post( - url_for('main.sign_in'), data={ - 'email_address': api_user_active.email_address, - 'password': 'val1dPassw0rd!'}) +def test_not_fresh_session_login_redirects_to_dashboard( + client, + api_user_active, + mock_login, + mock_get_user_by_email, + mock_verify_password, + mock_get_services_with_one_service +): + client.login(api_user_active) + with client.session_transaction() as session: + assert session['_fresh'] + session['_fresh'] = False + # This should skip the two factor + response = client.post( + url_for('main.sign_in'), data={ + 'email_address': api_user_active.email_address, + 'password': 'val1dPassw0rd!'}) + assert response.status_code == 302 + service_dct = mock_get_services_with_one_service(api_user_active.id)['data'][0] + assert response.location == url_for( + 'main.service_dashboard', service_id=service_dct['id'], _external=True) + + def test_not_fresh_session_login_redirects_to_choose_service( + client, + api_user_active, + mock_login, + mock_get_user_by_email, + mock_verify_password, + mock_get_services + ): + client.login(api_user_active) + with client.session_transaction() as session: + assert session['_fresh'] + session['_fresh'] = False + # This should skip the two factor + response = client.post( + url_for('main.sign_in'), data={ + 'email_address': api_user_active.email_address, + 'password': 'val1dPassw0rd!'}) assert response.status_code == 302 - service_dct = mock_get_services_with_one_service(api_user_active.id)['data'][0] - assert response.location == url_for( - 'main.service_dashboard', service_id=service_dct['id'], _external=True) + assert response.location == url_for('main.choose_service', _external=True) diff --git a/tests/conftest.py b/tests/conftest.py index e9f7cc8f9..93a9c8813 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -67,9 +67,7 @@ def fake_uuid(): @pytest.fixture(scope='function') def mock_get_service(mocker, api_user_active): def _get(service_id): - service = service_json( - service_id, "Test Service", [api_user_active.id], message_limit=50, - active=False, restricted=True) + service = service_json(service_id, users=[api_user_active.id], message_limit=50) return {'data': service} return mocker.patch('app.service_api_client.get_service', side_effect=_get) @@ -125,10 +123,7 @@ def mock_get_live_service(mocker, api_user_active): def _get(service_id): service = service_json( service_id, - "Test Service", - [api_user_active.id], - message_limit=1000, - active=False, + users=[api_user_active.id], restricted=False) return {'data': service} @@ -137,10 +132,9 @@ def mock_get_live_service(mocker, api_user_active): @pytest.fixture(scope='function') def mock_create_service(mocker): - def _create(service_name, active, message_limit, restricted, user_id, email_from): + def _create(service_name, message_limit, restricted, user_id, email_from): service = service_json( - 101, service_name, [user_id], message_limit=message_limit, - active=active, restricted=restricted, email_from=email_from) + 101, service_name, [user_id], message_limit=message_limit, restricted=restricted, email_from=email_from) return service['id'] return mocker.patch( @@ -152,7 +146,7 @@ def mock_update_service(mocker): def _update(service_id, **kwargs): service = service_json( service_id, - **{key: kwargs.get(key) for key in [ + **{key: kwargs[key] for key in kwargs if key in [ 'name', 'users', 'message_limit', @@ -193,7 +187,7 @@ def mock_get_services(mocker, fake_uuid, user=None): if user is None: user = active_user_with_permissions(fake_uuid) - def _create(user_id=None): + def _get_services(params_dict=None): service_one = service_json( SERVICE_ONE_ID, "service_one", [user.id], 1000, True, False) service_two = service_json( @@ -201,7 +195,7 @@ def mock_get_services(mocker, fake_uuid, user=None): return {'data': [service_one, service_two]} return mocker.patch( - 'app.service_api_client.get_services', side_effect=_create) + 'app.service_api_client.get_services', side_effect=_get_services) @pytest.fixture(scope='function') @@ -209,11 +203,11 @@ def mock_get_services_with_no_services(mocker, fake_uuid, user=None): if user is None: user = active_user_with_permissions(fake_uuid) - def _create(user_id=None): + def _get_services(params_dict=None): return {'data': []} return mocker.patch( - 'app.service_api_client.get_services', side_effect=_create) + 'app.service_api_client.get_services', side_effect=_get_services) @pytest.fixture(scope='function') @@ -221,13 +215,13 @@ def mock_get_services_with_one_service(mocker, fake_uuid, user=None): if user is None: user = api_user_active(fake_uuid) - def _create(user_id=None): + def _get_services(params_dict=None): return {'data': [service_json( SERVICE_ONE_ID, "service_one", [user.id], 1000, True, False )]} return mocker.patch( - 'app.service_api_client.get_services', side_effect=_create) + 'app.service_api_client.get_services', side_effect=_get_services) @pytest.fixture(scope='function') @@ -831,7 +825,7 @@ def mock_login(mocker, mock_get_user, mock_update_user, mock_events): def _verify_code(user_id, code, code_type): return True, '' - def _no_services(user_id=None): + def _no_services(params_dict=None): return {'data': []} return (