Skip to content
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.

Sign up for GitHub

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

Fix #674: Fix resource count on project dashboard #721

Merged
merged 2 commits into from
Sep 23, 2016
Merged

Fix #674: Fix resource count on project dashboard #721

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9ee76d8
Fix #674: Fix resource count on project dashboard
seav Sep 23, 2016
410665b
Resolve review feedback
seav Sep 23, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 31 additions & 2 deletions cadasta/organization/tests/test_views_default_projects.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,12 @@
from questionnaires.tests.factories import QuestionnaireFactory
from questionnaires.tests.utils import get_form
from questionnaires.models import Questionnaire
from resources.tests.factories import ResourceFactory
from resources.tests.utils import clear_temp # noqa
from resources.utils.io import ensure_dirs
from spatial.serializers import SpatialUnitGeoJsonSerializer
from spatial.tests.factories import SpatialUnitFactory
from party.tests.factories import PartyFactory

from .. import forms
from ..views import default
Expand Down Expand Up @@ -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()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK. I've reverted setup_template_context() and moved the test-specific context to render_content().

num_parties = self.project.parties.count()
num_resources = self.project.resource_set.filter(
archived=False).count()
return {
'object': self.project,
'project': self.project,
'geojson': '{"type": "FeatureCollection", "features": []}',
'geojson': json.dumps(SpatialUnitGeoJsonSerializer(
self.project.spatial_units.all(), many=True).data),
'is_superuser': False,
'is_administrator': False
'is_administrator': False,
'has_content': (
num_locations > 0 or num_parties > 0 or num_resources > 0),
'num_locations': num_locations,
'num_parties': num_parties,
'num_resources': num_resources,
}

def setup_url_kwargs(self):
Expand Down Expand Up @@ -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)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've removed the extra parties and location.

SpatialUnitFactory.create(project=self.project)
PartyFactory.create(project=self.project)
PartyFactory.create(project=self.project)
PartyFactory.create(project=self.project)
ResourceFactory.create(project=self.project)
ResourceFactory.create(project=self.project, archived=True)
response = self.request(user=self.user)
assert response.status_code == 200
assert response.content == self.expected_content
assert "<div class=\"num\">2</div> locations" in response.content
assert "<div class=\"num\">3</div> parties" in response.content
assert "<div class=\"num\">1</div> resource" in response.content


@pytest.mark.usefixtures('make_dirs')
class ProjectAddTest(UserTestCase, TestCase):
Expand Down
2 changes: 1 addition & 1 deletion cadasta/organization/views/default.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -361,7 +361,7 @@ def get_context_data(self, **kwargs):
context = super(ProjectDashboard, self).get_context_data(**kwargs)
num_locations = self.object.spatial_units.count()
num_parties = self.object.parties.count()
num_resources = self.object.resource_set.count()
num_resources = self.object.resource_set.filter(archived=False).count()
context['has_content'] = (
num_locations > 0 or num_parties > 0 or num_resources > 0)
context['num_locations'] = num_locations
Expand Down