CB-231: Retrieve data from database directly for browse review pages #155
Conversation
Modify jinja2 filter and add code for retrieving data for browse review pages
ferbncode
force-pushed
the
browse-pages
branch
from
30850c2
to
31c407f
Compare
| entites: List of tuples containing the entity_id and the entity_type. | ||
|
|
||
| Returns: | ||
| Dictionary containing the entities keyed by their mbid. |
There was a problem hiding this comment.
Can you also mention what the "entity" in the output actually is? Perhaps describing the output for each type is unnecessary, but at least mentioning what kind of information is expected there and what that depends on might be nice.
| release_group_mbids = [entity[0] for entity in filter(lambda entity: entity[1] == 'release_group', entities)] | ||
| place_mbids = [entity[0] for entity in filter(lambda entity: entity[1] == 'place', entities)] | ||
| event_mbids = [entity[0] for entity in filter(lambda entity: entity[1] == 'event', entities)] | ||
| if release_group_mbids: |
There was a problem hiding this comment.
This and other checks below seem redundant. If an empty list is passed we expect an empty output anyway, right?
| release_group_mbids, | ||
| includes=['artists'], | ||
| ) | ||
| entities_info.update(release_groups) |
There was a problem hiding this comment.
Also, you can just chain calls. No need to assign a variable here I think.
|
|
||
|
|
||
| def get_multiple_entities(entities): | ||
| """Fetch multiple entities using their mbids. |
| """Fetch multiple entities using their mbids. | ||
|
|
||
| Args: | ||
| entites: List of tuples containing the entity_id and the entity_type. |
There was a problem hiding this comment.
Why entity_id and entity_type and not simply entity ID and entity type? It would make sense to, for example, reference dictionary keys exactly by name, but in this case I don't see any point.
| entites: List of tuples containing the entity_id and the entity_type. | ||
|
|
||
| Returns: | ||
| Dictionary containing the basic information related to the entites. |
| from flask import current_app | ||
|
|
||
|
|
||
| def mock_get_entity_by_id(id, type='release_group'): # pylint: disable=unused-argument |
There was a problem hiding this comment.
id was unused as all tests in test_review.py use same entity ID for each type. I have fixed this as there might be other entities which may be tested in future. Thanks for pointing it out.
Modifed
jinja2filter for fetching entities by id and added code for retrieving data directly from database for browse review (/review) pages.