Merge lp:~mhall119/loco-team-portal/fixes-805280 into lp:loco-team-portal

[Michael Hall]

Fix various errors with using country name as part of the venue URL

Overview
========
Reverse lookup was failing on a Korean venue

Details
=======
Reverse lookups were failing because the Country object of Korea has the name "Korea, Republic of" and the comma was causing the error. I introduce a "slug" property to the Country model that will give a safer version of the country name, stripping commas and replacing spaces with underscores.

[Chris Johnston (cjohnston)]

Needs test cases.

review: Needs Fixing

[Chris Johnston (cjohnston)]

Works great.

[lp:~mhall119/loco-team-portal/fixes-805280]

450. By Michael Hall on 2011-07-12

[r=chrisjohnston][] Reduces the number of DB hits and code process for the teams list.

452. By Michael Hall on 2011-07-04

Replace hyphen with underscore in Country.slug fallback

453. By Michael Hall on 2011-07-12

Add tests

[Chris Johnston (cjohnston)]

When running ./manage.py test I got:

http://paste.ubuntu.com/645081/

review: Needs Information

[Michael Hall (mhall119)]

The Teams tests are old, I'll look into fixing them up later. For now, just run "python manage.py test venes" to run just the tests I added in this branch.

[Chris Johnston (cjohnston)]

Looks good.

review: Approve

=== modified file 'loco_directory/teams/models.py'
--- loco_directory/teams/models.py	2011-06-24 15:25:54 +0000
+++ loco_directory/teams/models.py	2011-07-12 00:57:31 +0000
@@ -59,6 +59,13 @@
     def related_venues(self):
         return self.venue_set.all()
 
+    @property
+    def slug(self):
+        if self.name is not None:
+            return self.name.replace(',', '').replace(' ', '_')
+        else:
+            return 'no_country'
+    
 def countries_without_continent():
     return Country.objects.filter(continents__isnull=True)
 
 
=== modified file 'loco_directory/templates/venues/venue_detail.html'
--- loco_directory/templates/venues/venue_detail.html	2011-03-16 18:35:53 +0000
+++ loco_directory/templates/venues/venue_detail.html	2011-07-12 00:57:31 +0000
@@ -11,7 +11,11 @@
 
 {% block sub_nav_links %}
 <a class="sub-nav-item" href="{% url venue-list %}" >{% trans "Back to Venues List" %}</a>
-<a class="sub-nav-item" href="{% url venue-update venue_object.country venue_object.id %}">{% trans "Edit Venue Details" %}</a>
+{% if venue_object.country %}
+<a class="sub-nav-item" href="{% url venue-update venue_object.country.slug venue_object.id %}">{% trans "Edit Venue Details" %}</a>
+{% else %}
+<a class="sub-nav-item" href="{% url venue-update 'no_country' venue_object.id %}">{% trans "Edit Venue Details" %}</a>
+{% endif %}
 {% endblock %}
 
 {% block content %}
 
=== modified file 'loco_directory/templates/venues/venue_list.html'
--- loco_directory/templates/venues/venue_list.html	2010-11-27 00:54:13 +0000
+++ loco_directory/templates/venues/venue_list.html	2011-07-12 00:57:31 +0000
@@ -57,7 +57,7 @@
                 <ul>
                     {% for venue in venues_without_country %}
                         <li>
-                            <p><a title="{% trans "show venue details" %}" href="{{ venue.get_absolute_url }}">{{ venue.name }}{% if venue.city %}, {{ venue.city }}{% endif %}{% if venue.spr %}, {{ venue.spr }}{% endif %}</a></p>
+                            <p><a title="{% trans "show venue details" %}" href="{{ venue.get_absolute_url }}">{% if venue %}{{ venue.name }}{% else %}{% trans 'No Name' %}{% endif %}{% if venue.city %}, {{ venue.city }}{% endif %}{% if venue.spr %}, {{ venue.spr }}{% endif %}</a></p>
                         </li>
                     {% endfor %}
                 </ul>
 
=== modified file 'loco_directory/venues/models.py'
--- loco_directory/venues/models.py	2011-06-17 17:10:41 +0000
+++ loco_directory/venues/models.py	2011-07-12 00:57:31 +0000
@@ -45,7 +45,11 @@
     @models.permalink
     def get_absolute_url(self):
         """ get the absolute url for the venue """
-        return ('venue-detail', [self.country or 'no-country', self.id])
+        if self.country:
+            country = self.country.slug
+        else:
+            country = 'no_country'
+        return ('venue-detail', [country, self.id])
 
     def save(self, *args, **kargs):
         if self.id:
 
=== modified file 'loco_directory/venues/tests.py'
--- loco_directory/venues/tests.py	2009-12-21 20:45:43 +0000
+++ loco_directory/venues/tests.py	2011-07-12 00:57:31 +0000
@@ -6,18 +6,52 @@
 """
 
 from django.test import TestCase
-
-class SimpleTest(TestCase):
-    def test_basic_addition(self):
-        """
-        Tests that 1 + 1 always equals 2.
-        """
-        self.failUnlessEqual(1 + 1, 2)
-
-__test__ = {"doctest": """
-Another way to test that 1 + 1 is equal to 2.
-
->>> 1 + 1 == 2
-True
-"""}
-
+from teams.models import Country, Team
+from venues.models import Venue
+
+
+class ViewTests(TestCase):
+
+    def setUp(self):
+        self.country = Country.objects.create(name='Test Country')
+        self.team = Team.objects.create(
+            lp_name='test-team',
+            name='Test Team',
+        )
+        self.team.countries.add(self.country)
+        self.venue = Venue.objects.create(
+            name='Test Venue',
+            country=self.country,
+        )
+
+    def test_venue_list(self):
+        response = self.client.get('/events/venues/')
+
+        # Check that the country group is labeled
+        self.assertContains(response, 'Test Country', 1)
+        
+        # Check that the venue name is displayed
+        self.assertContains(response, 'Test Venue', 1)
+        
+        # Check that the venue details URL exists using the propert country slug
+        self.assertContains(response, 'href="/events/venues/Test_Country/%s/detail/"' % self.venue.id, 1)
+
+    def test_venue_details_subnav(self):
+        response = self.client.get('/events/venues/Test_Country/%s/detail/' % self.venue.id)
+
+        # Check edit url exists using the proper country slug
+        self.assertContains(response, 'href="/events/venues/Test_Country/%s/update/"' % self.venue.id)
+
+    def test_country_contains_comma(self):
+        self.country.name='Korea, Republic of'
+        self.country.save()
+
+        # Check list view
+        response = self.client.get('/events/venues/')
+        self.assertContains(response, 'Korea, Republic of', 1)
+        self.assertContains(response, 'href="/events/venues/Korea_Republic_of/%s/detail/"' % self.venue.id, 1)
+
+        # Check details view
+        response = self.client.get('/events/venues/Test_Country/%s/detail/' % self.venue.id)
+        self.assertContains(response, 'href="/events/venues/Korea_Republic_of/%s/update/"' % self.venue.id)
+