From 12a83ce4c01843eafd27f9781ebe63f0511c9230 Mon Sep 17 00:00:00 2001
From: Max Erenberg <merenber@csclub.uwaterloo.ca>
Date: Thu, 19 Aug 2021 05:11:22 +0000
Subject: [PATCH] remove create_sync_response

---
 ceod/api/members.py            | 21 ++++++++++++---------
 ceod/api/utils.py              | 20 --------------------
 tests/ceod/api/test_members.py |  2 ++
 3 files changed, 14 insertions(+), 29 deletions(-)

diff --git a/ceod/api/members.py b/ceod/api/members.py
index 7a9453d..ba504f8 100644
--- a/ceod/api/members.py
+++ b/ceod/api/members.py
@@ -3,13 +3,12 @@ from zope import component
 
 from .utils import authz_restrict_to_staff, authz_restrict_to_syscom, \
     user_is_in_group, requires_authentication_no_realm, \
-    create_streaming_response, create_sync_response, development_only
-from ceo_common.errors import UserNotFoundError
+    create_streaming_response, development_only
+from ceo_common.errors import UserNotFoundError, BadRequest
 from ceo_common.interfaces import ILDAPService
 from ceod.transactions.members import (
     AddMemberTransaction,
     ModifyMemberTransaction,
-    RenewMemberTransaction,
     DeleteMemberTransaction,
 )
 import ceod.utils as utils
@@ -68,12 +67,16 @@ def patch_user(auth_user: str, username: str):
 @authz_restrict_to_staff
 def renew_user(username: str):
     body = request.get_json(force=True)
-    txn = RenewMemberTransaction(
-        username,
-        terms=body.get('terms'),
-        non_member_terms=body.get('non_member_terms'),
-    )
-    return create_sync_response(txn)
+    ldap_srv = component.getUtility(ILDAPService)
+    user = ldap_srv.get_user(username)
+    if body.get('terms'):
+        user.add_terms(body['terms'])
+        return {'terms_added': body['terms']}
+    elif body.get('non_member_terms'):
+        user.add_non_member_terms(body['non_member_terms'])
+        return {'non_member_terms_added': body['non_member_terms']}
+    else:
+        raise BadRequest('Must specify either terms or non-member terms')
 
 
 @bp.route('/<username>/pwreset', methods=['POST'])
diff --git a/ceod/api/utils.py b/ceod/api/utils.py
index 53282ea..97bc484 100644
--- a/ceod/api/utils.py
+++ b/ceod/api/utils.py
@@ -7,7 +7,6 @@ import traceback
 from typing import Callable, List
 
 from flask import current_app, stream_with_context
-from flask.json import jsonify
 from flask_kerberos import requires_authentication
 
 from ceo_common.logger_factory import logger_factory
@@ -107,25 +106,6 @@ def create_streaming_response(txn: AbstractTransaction):
         stream_with_context(generate()), mimetype='text/plain')
 
 
-def create_sync_response(txn: AbstractTransaction):
-    """
-    Runs the transaction synchronously and returns a JSON response.
-    """
-    try:
-        txn.execute()
-        # if the result is already an Object, don't wrap it again
-        if isinstance(txn.result, dict) or isinstance(txn.result, list):
-            return jsonify(txn.result)
-        # if the result is a string or number, wrap it in an Object
-        return {'result': txn.result}
-    except Exception as err:
-        logger.warning('Transaction failed:\n' + traceback.format_exc())
-        txn.rollback()
-        return {
-            'error': str(err),
-        }, 500
-
-
 def development_only(f: Callable) -> Callable:
     @functools.wraps(f)
     def wrapper(*args, **kwargs):
diff --git a/tests/ceod/api/test_members.py b/tests/ceod/api/test_members.py
index 863abdb..d7f72be 100644
--- a/tests/ceod/api/test_members.py
+++ b/tests/ceod/api/test_members.py
@@ -171,11 +171,13 @@ def test_api_renew_user(cfg, client, create_user_result, ldap_conn):
     status, data = client.post(
         f'/api/members/{uid}/renew', json={'terms': new_terms})
     assert status == 200
+    assert data == {'terms_added': new_terms}
 
     new_non_member_terms = ['w2022', 's2022']
     status, data = client.post(
         f'/api/members/{uid}/renew', json={'non_member_terms': new_non_member_terms})
     assert status == 200
+    assert data == {'non_member_terms_added': new_non_member_terms}
 
     # check that the changes were applied
     _, data = client.get(f'/api/members/{uid}')