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

Changes the get_institutions query to check for domain equality instead of a 'like' query #59

Conversation

jcadam14
Copy link
Contributor

@jcadam14 jcadam14 commented Dec 1, 2023

Updates the pytest in test_institutions_repo.test_get_institutions_by_domain to check for domain equality and to verify generic text strings won't find domains

Domain query will use the exact user domain instead of a generic text string like 'bank'. This forces that concept.

…ad of a 'like' query

Updates the pytest in test_institutions_repo.test_get_institutions_by_domain to check for
domain equality and to verify generic text strings won't find domains

Domain query will use the exact user domain instead of a generic text string like 'bank'.
This forces that concept.
@jcadam14 jcadam14 self-assigned this Dec 1, 2023
@jcadam14 jcadam14 linked an issue Dec 1, 2023 that may be closed by this pull request
Copy link

github-actions bot commented Dec 1, 2023

Coverage report

The coverage rate went from 84.32% to 84.29% ⬇️
The branch rate is 85%.

100% of new lines are covered.

Diff Coverage details (click to unfold)

src/entities/repos/institutions_repo.py

100% of new lines are covered (96.77% of the complete file).

…institutions_repo.py with black this time

black linter in GitHub actions was failing
@jcadam14 jcadam14 requested a review from hkeeler December 1, 2023 19:11
…the join to not be returned

Tried to "pretty" the code but associating the join at the start.  However this caused a query
for all institutions to only return those where the inner join didn't come back empty.
Added an extra institution to make the query for institutions a little
more comprehensive.  Because of the bug, the test_add_institution was
providing a false positive with returned banks == 3 because the join
was causing the newly added bank that did not have a domain to not
be returned.
Copy link
Collaborator

@lchen-2101 lchen-2101 left a comment

Choose a reason for hiding this comment

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

LGTM

@lchen-2101 lchen-2101 merged commit 6385c04 into main Dec 5, 2023
3 checks passed
@lchen-2101 lchen-2101 deleted the 56-update-institution-retrieval-with-domain-search-to-be-exact-match branch December 5, 2023 22:24
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.

Update institution retrieval with domain search to be exact match
2 participants