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

Conversation

ayobi
Copy link
Contributor

@ayobi ayobi commented Feb 5, 2024

Added delete_reason column and adjusted params allowing a delete reason to be attached from the admin when an account has been deleted.

@@ -14,6 +16,31 @@ 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'],

@@ -17,5 +17,6 @@ CREATE TABLE ag.account_removal_log (
-- user makes multiple requests.
disposition DISPOSITION_TYPE,
requested_on timestamptz,
reviewed_on timestamptz default current_timestamp
reviewed_on timestamptz default current_timestamp,
Copy link
Member

Choose a reason for hiding this comment

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

This is patching a patch, rather than a new patch?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, yeah I should make a new patch, better practice

@ayobi ayobi closed this Feb 8, 2024
@ayobi ayobi reopened this Feb 9, 2024
@ayobi
Copy link
Contributor Author

ayobi commented Feb 9, 2024

Sorry about that, all is good.

@ayobi ayobi changed the base branch from master-overhaul to master February 9, 2024 01:32
@cassidysymons
Copy link
Collaborator

@ayobi Can you please rename 0136.sql to 0137.sql in your working branch? A newly merged PR created a conflict there. Thanks!

@cassidysymons
Copy link
Collaborator

@ayobi When you have a moment, can you please merge biocore/microsetta-private-api:master into your working branch (ayobi:master-overhaul)? The build on our staging server fails if there are database patches missing. Thanks.

@ayobi
Copy link
Contributor Author

ayobi commented Feb 14, 2024

Should be merged now @cassidysymons 👍

@cassidysymons
Copy link
Collaborator

@ayobi Thanks for the quick turnaround on the interested user scrubbing, but can you break that into a separate branch (and pull request)? I already have the account deletion staging changes up on our staging server and I'd prefer to not expand scope here.

@ayobi
Copy link
Contributor Author

ayobi commented Feb 15, 2024

Absolute, will do 👍

@cassidysymons cassidysymons merged commit 5fd0c39 into biocore:master Apr 2, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants