-
Notifications
You must be signed in to change notification settings - Fork 3
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
Refactor: models #148
Merged
Merged
Refactor: models #148
Changes from 9 commits
Commits
Show all changes
10 commits
Select commit
Hold shift + click to select a range
d5e3b8a
refactor(models): split out eligibility types from User model
angela-tran a9f066d
refactor(init-db): remove usage of ast module
angela-tran 0223b78
feat(init-db): allow import file to have multiple rows for a given user
angela-tran 14f29fe
test(init-db): add test hard-coded to sample CSV file
angela-tran 887af3b
test(drop-db): add test
angela-tran 84eaacf
fix(drop-db): make sure drop-db still works on older databases
angela-tran 5cdb415
chore: update comment about the data type for 'types' parameter
angela-tran 7ce18bf
chore(pre-commit): autofix run
pre-commit-ci[bot] 9d714b8
fix(tests): short-term solution to ensure database tests are idempotent
angela-tran 7e64a94
test: document reason for resetting the database
angela-tran File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,7 @@ | ||
A1234567;Garcia;['type1'] | ||
B2345678;Hernandez;['type2'] | ||
C3456789;Smith;[] | ||
D4567890;Jones;['type1', 'type2'] | ||
a93ef111829829ea5038398ba6583dc2e1a2f2c309e24aee535f725dbe1f1250;2c3aaefea8267c66822f6edc0e42d9b7384695f9c0407eabda141770aab8901e;['type1'] | ||
095feaba889222ee1bc97aaf3bb89c90c21238fb556fb48b5cc54760461ea18ce814d016e70b1929d05190edbec1032e07921228fcb61ad3417aaff0abd9d082;123c86e1f2ac255ba31f1ad742defe23d194269669d2aac0d2572e20e9378e395976f84db305caeba1f91e7996463031d4c49365a7a9f4c7dc404873ad330974;['type1'] | ||
A1234567;Garcia;type1 | ||
B2345678;Hernandez;type2 | ||
C3456789;Smith; | ||
D4567890;Jones;type1 | ||
D4567890;Jones;type2 | ||
a93ef111829829ea5038398ba6583dc2e1a2f2c309e24aee535f725dbe1f1250;2c3aaefea8267c66822f6edc0e42d9b7384695f9c0407eabda141770aab8901e;type1 | ||
095feaba889222ee1bc97aaf3bb89c90c21238fb556fb48b5cc54760461ea18ce814d016e70b1929d05190edbec1032e07921228fcb61ad3417aaff0abd9d082;123c86e1f2ac255ba31f1ad742defe23d194269669d2aac0d2572e20e9378e395976f84db305caeba1f91e7996463031d4c49365a7a9f4c7dc404873ad330974;type1 |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,11 +1,26 @@ | ||
from eligibility_server.db import db | ||
|
||
|
||
user_eligibility = db.Table( | ||
"user_eligibility", | ||
db.Column("user_id", db.Integer, db.ForeignKey("user.id"), primary_key=True), | ||
db.Column("eligibility_id", db.Integer, db.ForeignKey("eligibility.id"), primary_key=True), | ||
) | ||
|
||
|
||
class Eligibility(db.Model): | ||
id = db.Column(db.Integer, primary_key=True) | ||
name = db.Column(db.String, unique=True, nullable=False) | ||
|
||
def __repr__(self): | ||
return f"<Eligibility {self.name}>" | ||
|
||
|
||
class User(db.Model): | ||
id = db.Column(db.Integer, primary_key=True) | ||
sub = db.Column(db.String, unique=True, nullable=False) | ||
name = db.Column(db.String, unique=True, nullable=False) | ||
types = db.Column(db.String, unique=False, nullable=False) | ||
types = db.relationship("Eligibility", secondary=user_eligibility, lazy="subquery", backref=db.backref("users", lazy=True)) | ||
|
||
def __repr__(self): | ||
return "<User %r>" % self.sub | ||
return f"<User {self.sub}>" |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Empty file.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,41 @@ | ||
import pytest | ||
|
||
from flask_sqlalchemy import inspect | ||
from eligibility_server.db import db | ||
from eligibility_server.db.models import Eligibility, User | ||
|
||
|
||
@pytest.mark.usefixtures("flask") | ||
def test_init_db_command(runner): | ||
"""Assumes that IMPORT_FILE_PATH is data/server.csv.""" | ||
runner.invoke(args="drop-db") | ||
|
||
result = runner.invoke(args="init-db") | ||
thekaveman marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
assert result.exit_code == 0 | ||
|
||
assert User.query.count() == 6 | ||
assert Eligibility.query.count() == 2 | ||
|
||
user_with_one_eligibility = User.query.filter_by(sub="A1234567", name="Garcia").first() | ||
type1_eligibility = Eligibility.query.filter_by(name="type1").first() | ||
assert user_with_one_eligibility.types == [type1_eligibility] | ||
|
||
user_with_no_eligibility = User.query.filter_by(sub="C3456789", name="Smith").first() | ||
assert user_with_no_eligibility.types == [] | ||
|
||
user_with_multiple_eligibilities = User.query.filter_by(sub="D4567890", name="Jones").first() | ||
type2_eligibility = Eligibility.query.filter_by(name="type2").first() | ||
assert user_with_multiple_eligibilities.types == [type1_eligibility, type2_eligibility] | ||
|
||
|
||
@pytest.mark.usefixtures("flask") | ||
def test_drop_db_command(runner): | ||
result = runner.invoke(args="drop-db") | ||
|
||
assert result.exit_code == 0 | ||
|
||
inspector = inspect(db.engine) | ||
assert inspector.get_table_names() == [] | ||
|
||
runner.invoke(args="init-db") | ||
angela-tran marked this conversation as resolved.
Show resolved
Hide resolved
|
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like how this is cleaner than the previous
str()
version.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do think it's a little funky to pass
user
and then useuser
as a key fordata
- not super obvious what is going on here?data
is a dict, might be clearer to iterate overdata.items()
to deal with the key/value tuple.Also this JSON file is... the oldest remnant of the old school approach from our toy. Not that important to maintain / make clearer right now, probably just drop it entirely in the future?
ALSO I've updated this section for the remote data anyway! So no worries.