From 0d350bdaf824bdc70c9487fb6bd8b368f2528f44 Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Tue, 16 Mar 2021 18:20:36 +0000 Subject: [PATCH] remove invited_user from session entirely now that we no longer set it since https://github.com/alphagov/notifications-admin/pull/3841 was merged, we don't need to remove it either. And we can remove checks that expect it when cleaning up the session. And the unit tests that make sure we ignore it if it's in the session. So long, session['invited_user'] and session['invited_org_user']! --- app/main/views/dashboard.py | 3 +- app/main/views/invites.py | 2 -- app/main/views/sign_in.py | 3 +- tests/app/main/views/test_accept_invite.py | 3 +- tests/app/models/test_user.py | 37 ---------------------- 5 files changed, 3 insertions(+), 45 deletions(-) diff --git a/app/main/views/dashboard.py b/app/main/views/dashboard.py index 1d43ba56a..ab940050f 100644 --- a/app/main/views/dashboard.py +++ b/app/main/views/dashboard.py @@ -49,8 +49,7 @@ def old_service_dashboard(service_id): @user_has_permissions() def service_dashboard(service_id): - if session.get('invited_user_id') or session.get('invited_user'): - session.pop('invited_user', None) + if session.get('invited_user_id'): session.pop('invited_user_id', None) session['service_id'] = service_id diff --git a/app/main/views/invites.py b/app/main/views/invites.py index 29ee21d1f..7704ff3fa 100644 --- a/app/main/views/invites.py +++ b/app/main/views/invites.py @@ -39,7 +39,6 @@ def accept_invite(token): service_name=service.name) if invited_user.status == 'accepted': - session.pop('invited_user', None) session.pop('invited_user_id', None) service = Service.from_id(invited_user.service) if service.has_permission('broadcast'): @@ -103,7 +102,6 @@ def accept_org_invite(token): organisation_name=organisation.name) if invited_org_user.status == 'accepted': - session.pop('invited_org_user', None) session.pop('invited_org_user_id', None) return redirect(url_for('main.organisation_dashboard', org_id=invited_org_user.organisation)) diff --git a/app/main/views/sign_in.py b/app/main/views/sign_in.py index f6e1a3598..d1a2c16d5 100644 --- a/app/main/views/sign_in.py +++ b/app/main/views/sign_in.py @@ -36,11 +36,10 @@ def sign_in(): if user and user.state == 'pending': return redirect(url_for('main.resend_email_verification', next=redirect_url)) - if user and (session.get('invited_user') or session.get('invited_user_id')): + if user and session.get('invited_user_id'): invited_user = InvitedUser.from_session() if user.email_address.lower() != invited_user.email_address.lower(): flash("You cannot accept an invite for another person.") - session.pop('invited_user', None) session.pop('invited_user_id', None) abort(403) else: diff --git a/tests/app/main/views/test_accept_invite.py b/tests/app/main/views/test_accept_invite.py index 3c6c100fc..24fa3bd7b 100644 --- a/tests/app/main/views/test_accept_invite.py +++ b/tests/app/main/views/test_accept_invite.py @@ -178,7 +178,7 @@ def test_accepting_invite_removes_invite_from_session( assert normalize_spaces(page.select_one('h1').text) == landing_page_title with client_request.session_transaction() as session: - assert 'invited_user' not in session + assert 'invited_user_id' not in session def test_existing_user_of_service_get_redirected_to_signin( @@ -416,7 +416,6 @@ def test_new_user_accept_invite_completes_new_registration_redirects_to_verify( with client.session_transaction() as session: assert response.status_code == 302 assert response.location == expected_redirect_location - assert 'invited_user' not in session assert session.get('invited_user_id') == USER_ONE_ID data = {'service': sample_invite['service'], diff --git a/tests/app/models/test_user.py b/tests/app/models/test_user.py index 1d0e044a5..ec7224e34 100644 --- a/tests/app/models/test_user.py +++ b/tests/app/models/test_user.py @@ -1,6 +1,3 @@ -import uuid -from unittest.mock import Mock - import pytest from app.models.user import AnonymousUser, InvitedOrgUser, InvitedUser, User @@ -121,22 +118,6 @@ def test_invited_user_from_session_uses_id(client, mocker, mock_get_invited_user mock_get_invited_user_by_id.assert_called_once_with(USER_ONE_ID) -def test_invited_user_from_session_uses_id_even_if_obj_in_session( - client, - mocker, - sample_invite, - mock_get_invited_user_by_id -): - mock_session_obj = Mock(spec=dict) - session_dict = {'invited_user_id': USER_ONE_ID, 'invited_user': mock_session_obj} - mocker.patch.dict('app.models.user.session', values=session_dict, clear=True) - - assert InvitedUser.from_session().id == USER_ONE_ID - - assert mock_session_obj.mock_calls == [] - mock_get_invited_user_by_id.assert_called_once_with(USER_ONE_ID) - - def test_invited_user_from_session_returns_none_if_nothing_present(client, mocker): mocker.patch.dict('app.models.user.session', values={}, clear=True) assert InvitedUser.from_session() is None @@ -151,24 +132,6 @@ def test_invited_org_user_from_session_uses_id(client, mocker, mock_get_invited_ mock_get_invited_org_user_by_id.assert_called_once_with(sample_org_invite['id']) -def test_invited_org_user_from_session_uses_id_even_if_obj_in_session( - client, - mocker, - sample_org_invite, - mock_get_invited_org_user_by_id -): - fake_id = str(uuid.uuid4()) - mock_org_dict = Mock(spec=dict) - session_dict = {'invited_org_user_id': fake_id, 'invited_org_user': mock_org_dict} - mocker.patch.dict('app.models.user.session', values=session_dict, clear=True) - - assert InvitedOrgUser.from_session().id == sample_org_invite['id'] - - # make sure we didn't access invited_org_user (as org_user_id takes precedence) - assert mock_org_dict.mock_calls == [] - mock_get_invited_org_user_by_id.assert_called_once_with(fake_id) - - def test_invited_org_user_from_session_returns_none_if_nothing_present(client, mocker): mocker.patch.dict('app.models.user.session', values={}, clear=True) assert InvitedOrgUser.from_session() is None