Fix #674: Fix resource count on project dashboard #721
Conversation
| @@ -202,12 +206,22 @@ def setup_models(self): | |||
| self.user.assign_policies(self.policy) | |||
|
|
|||
| def setup_template_context(self): | |||
| num_locations = self.project.spatial_units.count() | |||
There was a problem hiding this comment.
There are four database requests in here that are not necessary to all but one of the tests in this test case. It's slowing down the test run. What I would do is to adapt the response context in self.render_content for that one test. It's quite easy to do, since you know how many entities you have added, you can just add constants there instead of querying the database.
There was a problem hiding this comment.
OK. I've reverted setup_template_context() and moved the test-specific context to render_content().
| @@ -363,6 +377,21 @@ def test_get_archived_project_with_org_admin(self): | |||
| assert response.status_code == 200 | |||
| assert response.content == self.render_content(is_administrator=True) | |||
|
|
|||
| def test_get_with_overview_stats(self): | |||
| SpatialUnitFactory.create(project=self.project) | |||
There was a problem hiding this comment.
Here you create many records, just to test if the correct count is displayed. The view uses .count() on the queryset, which is a Django method; let's just assume that it works and is well-tested. I can see the case of testing whether .count() is used on all of the querysets in the view and the number is added correctly to the context, but you don't need three parties for that. I would get rid of at on spatial unit and two parties.
There was a problem hiding this comment.
I've removed the extra parties and location.
* Fix Cadasta#674: Fix resource count on project dashboard * Resolve review feedback
Proposed changes in this pull request
filter(archived=False).ProjectDashboardTestunit test class to complete the template context.When should this PR be merged
Anytime.
Risks
None foreseen.
Follow up actions
None.