fix: set the task field during populate missing tasks for detached dao #80
Conversation
lchen-2101
requested review from
jcadam14,
hkeeler,
guffee23 and
nargis-sultani
Coverage report
Click to see where and how coverage changed
This report was generated by python-coverage-comment-action |
||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Mainly question about the if in the pytest. And I'm thinking maybe a story at some point to rename some of the objects if you also think they're confusing (maybe it's just me coming back from vacation).
I thought for some reason that SQLAlchmey would handle populating the objects in tasks just be setting the task_names, it would handle the relationship building.
| assert res1.lei == "1234567890" | ||
| assert len(res1.tasks) == 2 | ||
| assert FilingTaskState.NOT_STARTED in set([t.state for t in res1.tasks]) | ||
| tasks1 = set([task_state.task for task_state in res1.tasks if task_state.task]) |
There was a problem hiding this comment.
Are the if's needed here? With your update, shouldn't be a None to check for should there?
Also, is it just me or did I name these DAO/DTOs poorly, now that I read it? FilingTaskState, FilingTask, etc. Just doesn't read well, even I get confused what objects are being held where and I fricking wrote it.
There was a problem hiding this comment.
ah yeap, if's not needed here, since if it's none, then the name wouldn't get populated either.
lol yeah, I'll come up with something more differentiating.
yeah, it's the attached / detached state; if we persisted the object, then the relationship probably would get built, but since it's just a newly constructed object, it doesn't try to acquire a connection to build it out is my educated guess. |
closes #79