CPHEAPM-26 - Add ToxRefDB vocab fixture and display #1066
Conversation
|
Hi @shapiromatron, marking this as ready for review. |
# Conflicts: # hawc/apps/vocab/api.py
There was a problem hiding this comment.
Fantastic job! I made some edits, but nothing too major. See my individual comments and then the diffs for the commits below; I tried to separate independent changes so you can hopefully follow along...
- 7754a21 cleanup abstract class (9 seconds ago)
- b6c44dd switch to guard check to return bad case, instead of check in component (20 minutes ago)
- f8cf5bb a few updates from testing (23 minutes ago)
- b9b742b remove script (move to another repo) (29 minutes ago)
- 89f0cff rename Toxref to ToxRefDB (2 hours ago)
- 238ca3c update migrations to be consistent with current main (2 hours ago)
- 868d8ac lazily generate large vocab dataframes instead of on app startup (2 hours ago)
- f95099d Merge branch 'main' into ICF-CPHEAPM26-toxref-vocab (4 hours ago)
| render() { | ||
| const tables = {ehv: EhvTable, toxrefdb: ToxRefDBTable}, | ||
| Table = tables[this.props.store.config.vocab]; | ||
| if (_.isUndefined(Table)) { |
There was a problem hiding this comment.
I changed this to guard (return w/ a different condition if we're not on the happy path)
| @@ -12,15 +13,17 @@ | |||
| from . import constants, models, serializers | |||
|
|
|||
|
|
|||
| class EhvTermViewSet(viewsets.GenericViewSet): | |||
| class VocabTermViewSet(viewsets.GenericViewSet): | |||
There was a problem hiding this comment.
nice job generalizing this. With your initial approach, the data was loaded every time the server restarted, which is all the time in development mode, since it was a class-level property on the class. I changed it to be a callable method, so it's only fetched when needed. I did the same for the view too.
Changes