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

@@ -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('User not in session')
+        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('User not in session')
         request = stored_request.to_request()
         send_rejected_mail(request, stored_request.reason)
         dispatch_event('ocflib.account_rejected', request=request.to_dict())

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()