From ef495d5fce7801e75386869f70f25d8cb813a4c9 Mon Sep 17 00:00:00 2001 From: kkuehler Date: Sat, 7 Jul 2018 15:36:41 -0700 Subject: [PATCH] approve: handle usernames not in session both of the added tests will fail prior to this patch. users of the approve_request and reject_request functions should catch a ValueError. fixes #81 --- ocflib/account/submission.py | 12 +++++++++--- tests/account/submission_test.py | 24 ++++++++++++++++++++++++ 2 files changed, 33 insertions(+), 3 deletions(-) diff --git a/ocflib/account/submission.py b/ocflib/account/submission.py index a23751dd..8b37bd0e 100644 --- a/ocflib/account/submission.py +++ b/ocflib/account/submission.py @@ -297,19 +297,25 @@ def get_remove_row_by_user_name(user_name): request_row = session.query(StoredNewAccountRequest).filter( StoredNewAccountRequest.user_name == user_name ).first() - session.delete(request_row) - session.commit() + if request_row: + session.delete(request_row) + session.commit() return request_row @celery_app.task def approve_request(user_name): - request = get_remove_row_by_user_name(user_name).to_request() + stored_request = get_remove_row_by_user_name(user_name) + if not stored_request: + raise ValueError('No pending request for user') + request = stored_request.to_request() create_account.delay(request) dispatch_event('ocflib.account_approved', request=request.to_dict()) @celery_app.task def reject_request(user_name): stored_request = get_remove_row_by_user_name(user_name) + if not stored_request: + raise ValueError('No pending request for user') request = stored_request.to_request() send_rejected_mail(request, stored_request.reason) dispatch_event('ocflib.account_rejected', request=request.to_dict()) diff --git a/tests/account/submission_test.py b/tests/account/submission_test.py index fde3fbc9..efc80155 100644 --- a/tests/account/submission_test.py +++ b/tests/account/submission_test.py @@ -123,6 +123,17 @@ def test_approve_request(celery_app, fake_new_account_request, session_with_requ ] +def test_approve_request_invalid(celery_app, fake_new_account_request, session_with_requests, tasks): + assert len(session_with_requests.query(StoredNewAccountRequest).all()) == 2 + with pytest.raises(ValueError): + tasks.approve_request('keur') + + # invalid request, nothing changed + assert len(session_with_requests.query(StoredNewAccountRequest).all()) == 2 + tasks.create_account.delay.assert_not_called() + assert celery_app._sent_messages == [] + + @mock.patch('ocflib.account.submission.send_rejected_mail') def test_reject_request(send_rejected_mail, celery_app, fake_new_account_request, session_with_requests, tasks): tasks.reject_request(fake_new_account_request.user_name) @@ -139,6 +150,19 @@ def test_reject_request(send_rejected_mail, celery_app, fake_new_account_request send_rejected_mail.assert_called_once_with(request, mock.ANY) +@mock.patch('ocflib.account.submission.send_rejected_mail') +def test_reject_request_invalid(send_rejected_mail, celery_app, fake_new_account_request, + session_with_requests, tasks): + assert len(session_with_requests.query(StoredNewAccountRequest).all()) == 2 + with pytest.raises(ValueError): + tasks.reject_request('keur') + + # invalid request, nothing changed + assert len(session_with_requests.query(StoredNewAccountRequest).all()) == 2 + assert celery_app._sent_messages == [] + send_rejected_mail.assert_not_called() + + def test_get_pending_requests(session_with_requests, tasks, fake_new_account_request): request = fake_new_account_request pending_requests = tasks.get_pending_requests()