Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

added admin delete reason #560

Merged
merged 6 commits into from
Apr 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions microsetta_private_api/admin/admin_impl.py
Original file line number Diff line number Diff line change
Expand Up @@ -885,15 +885,16 @@ def ignore_removal_request(account_id, token_info):
try:
# remove the user from the queue, noting the admin who allowed it
# and the time the action was performed.
rq_repo.update_queue(account_id, token_info['email'], 'ignored')
rq_repo.update_queue(account_id, token_info['email'],
'ignored', None)
t.commit()
except RepoException as e:
raise e

return None, 204


def allow_removal_request(account_id, token_info):
def allow_removal_request(account_id, token_info, delete_reason):
validate_admin_access(token_info)

with Transaction() as t:
Expand All @@ -902,7 +903,8 @@ def allow_removal_request(account_id, token_info):
try:
# remove the user from the queue, noting the admin who allowed it
# and the time the action was performed.
rq_repo.update_queue(account_id, token_info['email'], 'deleted')
rq_repo.update_queue(account_id, token_info['email'],
'deleted', delete_reason)
t.commit()
except RepoException as e:
raise e
Expand Down
4 changes: 2 additions & 2 deletions microsetta_private_api/api/_removal_queue.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,13 @@ def check_request_remove_account(account_id, token_info):
return jsonify(result), 200


def request_remove_account(account_id, token_info):
def request_remove_account(account_id, token_info, user_delete_reason=None):
# raises 401 if method fails
_validate_account_access(token_info, account_id)

with Transaction() as t:
rq_repo = RemovalQueueRepo(t)
rq_repo.request_remove_account(account_id)
rq_repo.request_remove_account(account_id, user_delete_reason)
t.commit()

return jsonify(code=200, message="Request Accepted"), 200
Expand Down
17 changes: 17 additions & 0 deletions microsetta_private_api/api/microsetta_private_api.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,12 @@ paths:
description: Request account to be removed
parameters:
- $ref: '#/components/parameters/account_id'
- name: user_delete_reason
in: query
description: Reason for account deletion
required: false
schema:
type: string
responses:
'200':
description: Successfully requested for account to be removed
Expand All @@ -199,6 +205,12 @@ paths:
description: Cancel request for account to be removed
parameters:
- $ref: '#/components/parameters/account_id'
- name: user_delete_reason
in: query
description: Reason for account deletion
required: false
schema:
type: string
responses:
'200':
description: Successfully canceled request
Expand Down Expand Up @@ -2504,6 +2516,11 @@ paths:
- Admin
parameters:
- $ref: '#/components/parameters/account_id'
- name: delete_reason
in: query
required: true
schema:
type: string
responses:
'200':
description: Updates queue, log before calling delete_account()
Expand Down
12 changes: 8 additions & 4 deletions microsetta_private_api/api/tests/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -1425,6 +1425,7 @@ def test_request_account_removal(self):
self.assertFalse(json.loads(response.data)['status'])

# submit a request for this account to be removed.
delete_reason = "User requested account removal"
response = self.client.put(
f'/api/accounts/{dummy_acct_id}/removal_queue',
headers=self.dummy_auth)
Expand All @@ -1440,8 +1441,9 @@ def test_request_account_removal(self):
headers=self.dummy_auth)

self.assertEqual(200, response.status_code)

self.assertTrue(json.loads(response.data)['status'])
removal_info = json.loads(response.data)
self.assertTrue(removal_info['status'])
# self.assertEqual(removal_info['delete_reason'], delete_reason)

# try to request a second time. Verify that an error is returned
# instead.
Expand Down Expand Up @@ -1491,7 +1493,8 @@ def test_request_account_removal(self):
"Request Accepted")

response = self.client.put(
f'/api/admin/account_removal/{dummy_acct_id}',
f'/api/admin/account_removal/{dummy_acct_id}'
f'?delete_reason={delete_reason}',
headers=make_headers(FAKE_TOKEN_ADMIN))

self.assertEqual(204, response.status_code)
Expand Down Expand Up @@ -1557,7 +1560,8 @@ def test_request_account_removal(self):
# functionality; it deletes the id from the delete-queue and logs the
# deletion in a separate 'log' table, before calling account_delete().
response = self.client.delete(
f'/api/admin/account_removal/{dummy_acct_id}',
f'/api/admin/account_removal/{dummy_acct_id}'
f'?delete_reason={delete_reason}',
headers=make_headers(FAKE_TOKEN_ADMIN))

# confirm that the operation was a success.
Expand Down
7 changes: 7 additions & 0 deletions microsetta_private_api/db/patches/0137.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
-- Feb 5, 2024
-- Add delete_reason to ag.account_removal_log
ALTER TABLE ag.account_removal_log
ADD COLUMN delete_reason VARCHAR;

COMMENT ON COLUMN ag.account_removal_log.delete_reason
IS 'Reason the admin gave for deleting the account.';
7 changes: 7 additions & 0 deletions microsetta_private_api/db/patches/0138.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
-- Feb 12, 2024
-- Add user_delete_reason to ag.delete_account_queue
ALTER TABLE ag.delete_account_queue
ADD COLUMN user_delete_reason VARCHAR;

COMMENT ON COLUMN ag.delete_account_queue.user_delete_reason
IS 'Reason the user gave for deleting the account.';
3 changes: 2 additions & 1 deletion microsetta_private_api/model/removal_queue_requests.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@

class RemovalQueueRequest(ModelBase):
def __init__(self, id, account_id, email, first_name, last_name,
requested_on):
requested_on, user_delete_reason):
self.id = id
self.account_id = account_id
self.email = email
Expand All @@ -12,6 +12,7 @@ def __init__(self, id, account_id, email, first_name, last_name,

# 2022-07-27 17:15:33.937458-07:00 -> 2022-07-27 17:15:33
self.requested_on = str(requested_on).split('.')[0]
self.user_delete_reason = user_delete_reason

def to_api(self):
return self.__dict__.copy()
49 changes: 42 additions & 7 deletions microsetta_private_api/repo/removal_queue_repo.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
from microsetta_private_api.repo.base_repo import BaseRepo
from microsetta_private_api.exceptions import RepoException
from microsetta_private_api.model.removal_queue_requests \
import RemovalQueueRequest


class RemovalQueueRepo(BaseRepo):
Expand All @@ -14,6 +16,32 @@ def _check_account_is_admin(self, admin_email):

return False if count == 0 else True

def _row_to_removal(self, r):
return RemovalQueueRequest(r['id'], r['account_id'], r['email'],
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In case this is called from a subclass, this is a little more defensive:

Suggested change
return RemovalQueueRequest(r['id'], r['account_id'], r['email'],
return self.__class__(r['id'], r['account_id'], r['email'],

r['first_name'], r['last_name'],
r['requested_on'], r['user_delete_reason'])

def get_all_account_removal_requests(self):
with self._transaction.dict_cursor() as cur:
cur.execute("""
SELECT
ag.delete_account_queue.id,
ag.delete_account_queue.account_id,
ag.delete_account_queue.requested_on,
ag.delete_account_queue.user_delete_reason,
ag.account.first_name,
ag.account.last_name,
ag.account.email
FROM
ag.account
JOIN
ag.delete_account_queue ON ag.account.id
= ag.delete_account_queue.account_id
""")
rows = cur.fetchall()

return [self._row_to_removal(r) for r in rows]

def check_request_remove_account(self, account_id):
with self._transaction.cursor() as cur:
cur.execute("SELECT count(id) FROM delete_account_queue WHERE "
Expand All @@ -22,7 +50,7 @@ def check_request_remove_account(self, account_id):

return False if count == 0 else True

def request_remove_account(self, account_id):
def request_remove_account(self, account_id, user_delete_reason):
with self._transaction.cursor() as cur:
cur.execute("SELECT account_id from delete_account_queue where "
"account_id = %s", (account_id,))
Expand All @@ -31,9 +59,13 @@ def request_remove_account(self, account_id):
if result is not None:
raise RepoException("Account is already in removal queue")

user_delete_reason_value = user_delete_reason \
if user_delete_reason else None

cur.execute(
"INSERT INTO delete_account_queue (account_id) VALUES (%s)",
(account_id,))
"INSERT INTO delete_account_queue (account_id, "
"user_delete_reason) VALUES (%s, %s)",
(account_id, user_delete_reason_value))

def cancel_request_remove_account(self, account_id):
if not self.check_request_remove_account(account_id):
Expand All @@ -43,7 +75,8 @@ def cancel_request_remove_account(self, account_id):
cur.execute("DELETE FROM delete_account_queue WHERE account_id ="
" %s", (account_id,))

def update_queue(self, account_id, admin_email, disposition):
def update_queue(self, account_id, admin_email,
disposition, delete_reason):
if not self.check_request_remove_account(account_id):
raise RepoException("Account is not in removal queue")

Expand All @@ -69,9 +102,11 @@ def update_queue(self, account_id, admin_email, disposition):
# add an entry to the log detailing who reviewed the account
# and when.
cur.execute("INSERT INTO account_removal_log (account_id, "
"admin_id, disposition, requested_on) VALUES (%s,"
" %s, %s, %s)", (account_id, admin_id, disposition,
requested_on))
"admin_id, disposition, requested_on, delete_reason) "
"VALUES (%s, %s, %s, %s, %s)", (account_id,
admin_id, disposition,
requested_on,
delete_reason))

# delete the entry from queue. Note that reviewed entries are
# deleted from the queue whether or not they were approved
Expand Down
35 changes: 19 additions & 16 deletions microsetta_private_api/repo/tests/test_removal_queue_repo.py
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ def test_check_request_remove_account(self):

# use request_remove_account() to push the valid account onto
# the queue.
rqr.request_remove_account(self.acc.id)
rqr.request_remove_account(self.acc.id, 'delete reason')

# assume request_remove_account() succeeded.
# check_request_remove_account() should return True.
Expand All @@ -120,7 +120,7 @@ def test_request_remove_account(self):
rqr = RemovalQueueRepo(t)
# use request_remove_account() to push the valid account onto
# the queue.
rqr.request_remove_account(self.acc.id)
rqr.request_remove_account(self.acc.id, 'delete reason')

# assume check_request_remove_account() works correctly.
# verify account is now in the queue.
Expand All @@ -131,20 +131,21 @@ def test_request_remove_account_failure(self):
rqr = RemovalQueueRepo(t)

# remove a valid account twice
rqr.request_remove_account(self.acc.id)
rqr.request_remove_account(self.acc.id, 'delete reason')
with self.assertRaises(RepoException):
rqr.request_remove_account(self.acc.id)
rqr.request_remove_account(self.acc.id, 'delete reason')

# remove a non-existant id.
with self.assertRaises(ForeignKeyViolation):
rqr.request_remove_account(RemovalQueueTests.bad_id)
rqr.request_remove_account(RemovalQueueTests.bad_id,
'delete reason')

def test_cancel_request_remove_account(self):
with Transaction() as t:
rqr = RemovalQueueRepo(t)
# use request_remove_account() to push the valid account onto
# the queue.
rqr.request_remove_account(self.acc.id)
rqr.request_remove_account(self.acc.id, 'delete reason')

# assume check_request_remove_account() works correctly.
# verify account is now in the queue.
Expand Down Expand Up @@ -177,7 +178,7 @@ def test_cancel_request_remove_account_failure(self):

# use request_remove_account() to push the valid account onto
# the queue.
rqr.request_remove_account(self.acc.id)
rqr.request_remove_account(self.acc.id, 'delete reason')

# cancel the request to delete the account twice.
rqr.cancel_request_remove_account(self.acc.id)
Expand All @@ -189,12 +190,13 @@ def test_update_queue_success(self):
rqr = RemovalQueueRepo(t)

# push the standard account onto the queue.
rqr.request_remove_account(self.acc.id)
rqr.request_remove_account(self.acc.id, 'delete reason')

# update_queue should migrate the relevant information to the
# ag.account_removal_log table and delete the entry from the
# queue table.
rqr.update_queue(self.acc.id, self.adm.email, 'deleted')
rqr.update_queue(self.acc.id, self.adm.email, 'deleted',
'delete reason')

# confirm that the account id is no longer in the queue table.
self.assertFalse(rqr.check_request_remove_account(self.acc.id))
Expand All @@ -205,16 +207,17 @@ def test_update_queue_success(self):
with Transaction() as t:
with t.cursor() as cur:
cur.execute("SELECT account_id, admin_id, disposition, "
"requested_on, reviewed_on FROM "
"requested_on, reviewed_on, delete_reason FROM "
"ag.account_removal_log")
rows = cur.fetchall()
self.assertEqual(len(rows), 1)
for account_id, admin_id, disposition, requested_on,\
reviewed_on in rows:
reviewed_on, delete_reason in rows:
# note this loop should only execute once.
self.assertEqual(account_id, self.acc.id)
self.assertEqual(admin_id, self.adm.id)
self.assertEqual(disposition, 'deleted')
self.assertEqual(delete_reason, 'delete reason')
now = datetime.datetime.now().timestamp()
# the requested_on time should be not far in the past.
# assume it is not NULL and is less than a minute ago.
Expand All @@ -227,23 +230,23 @@ def test_update_queue_failure(self):
rqr = RemovalQueueRepo(t)

with self.assertRaises(InvalidTextRepresentation):
rqr.update_queue('XXXX', self.adm.email, 'ignored')
rqr.update_queue('XXXX', self.adm.email, 'ignored', None)

with Transaction() as t:
rqr = RemovalQueueRepo(t)

# push the standard account onto the queue.
rqr.request_remove_account(self.acc.id)
rqr.request_remove_account(self.acc.id, 'delete reason')

# ensure that an Error is raised when an invalid admin
# email address is passed.
with self.assertRaises(RepoException):
rqr.update_queue(self.acc.id, 'XXXX', 'ignored')
rqr.update_queue(self.acc.id, 'XXXX', 'ignored', None)

# ensure that an Error is raised when disposition is None or
# emptry string.
with self.assertRaises(RepoException):
rqr.update_queue(self.acc.id, self.adm.email, None)
rqr.update_queue(self.acc.id, self.adm.email, None, None)

with self.assertRaises(RepoException):
rqr.update_queue(self.acc.id, self.adm.email, '')
rqr.update_queue(self.acc.id, self.adm.email, '', None)
Loading