don't swallow HTTP errors from create_event

tests weren't patching out create_event (which is invoked every time a
user logs in). This was getting caught by our egress proxy on jenkins.
We didn't notice because the event handler code was swallowing all
exceptions and not re-raising.

This changes that code to no longer swallow exceptions. Since we did
that, we also need to update all the tests that test log-in to mock
the call

app/event_handlers.py

@@ -1,3 +1,5 @@
+from flask import request
+
 from app import events_api_client
 
 
@@ -6,17 +8,15 @@ def on_user_logged_in(sender, user):
 
 
 def _send_event(sender, **kwargs):
-    from flask import request
+    if not kwargs.get('event_type'):
-    try:
+        return
-        event_data = _construct_event_data(request)
+
-        if kwargs.get('user'):
+    event_data = _construct_event_data(request)
-            event_data['user_id'] = kwargs.get('user').id
+
-        if not kwargs.get('event_type'):
+    if kwargs.get('user'):
-            return
+        event_data['user_id'] = kwargs.get('user').id
-        events_api_client.create_event(kwargs['event_type'], event_data)
+
-    except Exception as e:
+    events_api_client.create_event(kwargs['event_type'], event_data)
-        sender.logger.error('Error creating event')
-        sender.logger.error(e)
 
 
 def _construct_event_data(request):

tests/__init__.py

@@ -1,3 +1,4 @@
+from unittest.mock import patch
 import pytest
 import uuid
 
@@ -18,12 +19,13 @@ class TestClient(FlaskClient):
             session['user_id'] = user.id
         if mocker:
             mocker.patch('app.user_api_client.get_user', return_value=user)
-            mocker.patch('app.events_api_client.create_event')
         if mocker and service:
             with self.session_transaction() as session:
                 session['service_id'] = service['id']
             mocker.patch('app.service_api_client.get_service', return_value={'data': service})
-        login_user(user)
+
+        with patch('app.events_api_client.create_event'):
+            login_user(user)
 
     def logout(self, user):
         self.get(url_for("main.logout"))

tests/app/main/views/test_accept_invite.py

@@ -373,6 +373,7 @@ def test_new_invited_user_verifies_and_added_to_service(
     mock_get_users_by_service,
     mock_get_detailed_service,
     mock_get_usage,
+    mock_create_event,
     mocker,
 ):
     # visit accept token page

tests/app/main/views/test_register.py

@@ -239,6 +239,7 @@ def test_register_from_email_auth_invite(
     mock_send_verify_email,
     mock_send_verify_code,
     mock_accept_invite,
+    mock_create_event,
 ):
     sample_invite['auth_type'] = 'email_auth'
     with client.session_transaction() as session:
@@ -287,6 +288,7 @@ def test_can_register_email_auth_without_phone_number(
     mock_send_verify_email,
     mock_send_verify_code,
     mock_accept_invite,
+    mock_create_event,
 ):
     sample_invite['auth_type'] = 'email_auth'
     with client.session_transaction() as session:

tests/app/main/views/test_two_factor.py

@@ -26,6 +26,7 @@ def test_should_login_user_and_should_redirect_to_next_url(
     mock_get_user,
     mock_get_user_by_email,
     mock_check_verify_code,
+    mock_create_event,
 ):
     with client.session_transaction() as session:
         session['user_details'] = {
@@ -48,6 +49,7 @@ def test_should_login_user_and_not_redirect_to_external_url(
     mock_get_user_by_email,
     mock_check_verify_code,
     mock_get_services_with_one_service,
+    mock_create_event,
 ):
     with client.session_transaction() as session:
         session['user_details'] = {
@@ -65,6 +67,7 @@ def test_should_login_user_and_redirect_to_show_accounts(
     mock_get_user,
     mock_get_user_by_email,
     mock_check_verify_code,
+    mock_create_event,
 ):
     with client.session_transaction() as session:
         session['user_details'] = {
@@ -100,6 +103,7 @@ def test_should_login_user_when_multiple_valid_codes_exist(
     mock_get_user_by_email,
     mock_check_verify_code,
     mock_get_services_with_one_service,
+    mock_create_event,
 ):
     with client.session_transaction() as session:
         session['user_details'] = {
@@ -117,6 +121,7 @@ def test_two_factor_should_set_password_when_new_password_exists_in_session(
     mock_check_verify_code,
     mock_get_services_with_one_service,
     mock_update_user_password,
+    mock_create_event,
 ):
     with client.session_transaction() as session:
         session['user_details'] = {
@@ -166,6 +171,7 @@ def test_two_factor_should_activate_pending_user(
     mocker,
     api_user_pending,
     mock_check_verify_code,
+    mock_create_event,
     mock_activate_user,
 ):
     mocker.patch('app.user_api_client.get_user', return_value=api_user_pending)
@@ -186,7 +192,8 @@ def test_valid_two_factor_email_link_logs_in_user(
     valid_token,
     mock_get_user,
     mock_get_services_with_one_service,
-    mocker
+    mocker,
+    mock_create_event,
 ):
     mocker.patch('app.user_api_client.check_verify_code', return_value=(True, ''))
 

tests/app/main/views/test_verify.py

@@ -32,7 +32,8 @@ def test_should_redirect_to_add_service_when_sms_code_is_correct(
     mocker,
     mock_update_user_attribute,
     mock_check_verify_code,
-    fake_uuid
+    mock_create_event,
+    fake_uuid,
 ):
     api_user_active.current_session_id = str(uuid.UUID(int=1))
     mocker.patch('app.user_api_client.get_user', return_value=api_user_active)
@@ -60,6 +61,7 @@ def test_should_activate_user_after_verify(
     api_user_pending,
     mock_send_verify_code,
     mock_check_verify_code,
+    mock_create_event,
     mock_activate_user,
 ):
     mocker.patch('app.user_api_client.get_user', return_value=api_user_pending)

tests/conftest.py

@@ -2923,3 +2923,14 @@ def mock_get_organisations_and_services_for_user(mocker, organisation_one, api_u
         'app.user_api_client.get_organisations_and_services_for_user',
         side_effect=_get_orgs_and_services
     )
+
+
+@pytest.fixture
+def mock_create_event(mocker):
+    """
+    This should be used whenever your code is calling `flask_login.login_user`
+    """
+    def _add_event(event_type, event_data):
+        return
+
+    return mocker.patch('app.events_api_client.create_event', side_effect=_add_event)