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 database constraint on AuthRocket #561

Merged
merged 4 commits into from
Feb 13, 2024

Conversation

colts661
Copy link
Contributor

@colts661 colts661 commented Feb 6, 2024

Fixed #559. The constraint error is handled in both create_account() and update_account(), and so the tests are built upon these two methods without committing in the tests.

@colts661
Copy link
Contributor Author

colts661 commented Feb 6, 2024

The 422 code test in test_api for create_account() directly addresses this issue but seems to have some extra legacy handling afterwards. Please let me know:

  1. What's expected in this test case after the addition of db constraint;
  2. Should I move all added tests to api/tests/test_api.py altogether.

Thank you!!

@cassidysymons
Copy link
Collaborator

@colts661 You can delete lines 898 - 911 in test_api.py. Since the new database constraint makes the condition impossible, it's no longer necessary to test what the API does if it happens.

I'll review the rest of the PR in the next couple of hours. Thanks!

@cassidysymons
Copy link
Collaborator

@colts661 When you have a moment, can you please rename 0135.sql to 0136.sql in your working branch? Another PR merge created a conflict. Thanks!

@cassidysymons
Copy link
Collaborator

@colts661 Can you please update your working branch from biocore/microsetta-private-api:master? Patch 0135.sql isn't in your working branch and is causing the build to fail when I test it on our staging server.

@cassidysymons cassidysymons merged commit 07a5757 into biocore:master Feb 13, 2024
2 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.

Database constraint on auth_issuer and auth_sub columns
2 participants