Merge lp:~toabctl/loco-team-portal/simplify_team_code into lp:~ltp-devs/loco-team-portal/trunk

[Thomas Bechtold]

[Thomas Bechtold (toabctl)]

This branch should simplify the team code.

* simplify team forms
* use django message system
* use django login decorator
* add initial unittest for team update
* use {% url %} in templates and revert() in views

[lp:~toabctl/loco-team-portal/simplify_team_code]

123. By Thomas Bechtold on 2009-12-20

remove unecessary teammanager method

124. By Thomas Bechtold on 2009-12-20

use more django-like url names and functions names

125. By Thomas Bechtold on 2009-12-20

fix url in index.html

126. By Thomas Bechtold on 2009-12-20

fix url in team_detail.html

127. By Thomas Bechtold on 2009-12-20

use {% url %} istead of get_absolute_url

128. By Thomas Bechtold on 2009-12-20

use {% url %} istead of get_absolute_url

129. By Thomas Bechtold on 2009-12-21

fix {% url %} and reverse() bugs because of new url names

[Daniel Holbach (dholbach)]

Great work.

review: Approve

=== modified file 'loco_directory/teams/forms.py'
--- loco_directory/teams/forms.py	2009-11-18 22:56:44 +0000
+++ loco_directory/teams/forms.py	2009-12-21 09:39:15 +0000
@@ -4,32 +4,12 @@
 
 from models import Team
 
-class TeamForm(forms.ModelForm):
-    def __init__(self, *args, **kwargs):
-        super(TeamForm, self).__init__(*args, **kwargs)
-        
-        team = self.instance
-        self.fields['country'].initial = team.country
-        if self.fields['country'] == "United States":
-            from django.contrib.localflavor.us.us_states import STATE_CHOICES
-            self.fields['spr'].choices = STATE_CHOICES
-        self.fields['spr'].initial = team.spr
-        self.fields['city'].initial = team.city
-        self.fields['wiki_url'].initial = team.wiki_url
-        self.fields['web_url'].initial = team.web_url
-        self.fields['ml_url'].initial = team.ml_url
-        self.fields['forum_url'].initial = team.forum_url
-        self.fields['email'].initial = team.email
-        self.fields['irc_chan'].initial = team.irc_chan
-        self.fields['provides_support'].initial = team.provides_support
-
+class UpdateTeamForm(forms.ModelForm):
     class Meta:
         model = Team
-
-class UpdateTeamForm(TeamForm):
-    class Meta(TeamForm.Meta):
         exclude = ('approved_date', 'approved', 'expires_date', 'name', 'lp_name', 'admins', 'owner')
 
-class LoCoCouncilForm(TeamForm):
-    class Meta(TeamForm.Meta):
+class LoCoCouncilForm(forms.ModelForm):
+    class Meta:
+        model = Team
         exclude = ('approved', 'expires_date', 'name', 'lp_name', 'admins', 'owner')

=== modified file 'loco_directory/teams/models.py'
--- loco_directory/teams/models.py	2009-12-11 13:47:44 +0000
+++ loco_directory/teams/models.py	2009-12-21 09:39:15 +0000
@@ -2,9 +2,7 @@
 from django.utils.translation import ugettext as _
 
 class TeamManager(models.Manager):
-    def all_teams(self):
-        teams = self.all()
-        return teams
+    pass
 
 class TeamAdministrator(models.Model):
     lpid = models.SlugField(_("Launchpad ID"), max_length=40, null=False, blank=False)
@@ -43,6 +41,3 @@
     def __unicode__(self):
         return self.lp_name
     
-    @models.permalink
-    def get_absolute_url(self):
-        return ('team_slug', [self.lp_name])

=== modified file 'loco_directory/teams/tests.py'
--- loco_directory/teams/tests.py	2009-11-12 09:13:44 +0000
+++ loco_directory/teams/tests.py	2009-12-21 09:39:15 +0000
@@ -3,6 +3,9 @@
 from teams.models import *
 from common import launchpad
 
+from django.core.urlresolvers import reverse
+
+
 class LoCoCouncilACLTestCase(TestCase):
     def testACLs(self):
         """Test if LoCo Council check works
@@ -26,3 +29,43 @@
         venezuela = teams.filter(lp_name="ubuntu-ve")[0]
         self.assertEquals(launchpad.is_admin_or_owner("effie-jayx", venezuela), True)
         self.assertEquals(launchpad.is_admin_or_owner("jorge", venezuela), False)
+
+class EditTeamTestCase(TestCase):
+    """
+    test if team update works correct
+    """
+    def test_anonymous_user(self):
+        """
+        test the team update as anonymous user (not logged in)
+        """
+        #try to access team-update page as not logged in user
+        response = self.client.get(reverse( 'team-edit', args=['ubuntu-de-locoteam'] ), follow=True)
+        self.assertRedirects(response, '/openid/login?next=/teams/ubuntu-de-locoteam/edit', status_code=302, target_status_code=200)
+        #try to update team as not logged in user
+        response = self.client.post(reverse('team-edit', args=['ubuntu-de-locoteam']), {'Country':'Germany', 'forum_url':'http://ubuntuusers.de',  })
+        self.assertRedirects(response, '/openid/login?next=/teams/ubuntu-de-locoteam/edit', status_code=302, target_status_code=200)
+    
+    def test_user_with_no_rights(self):
+        """
+        test the team update with a user which has no rights
+        """
+        pass
+        #FIXME How to use a user with no right with openid?
+    
+    def test_user_loco_council(self):
+        """
+        test the team update with a user which is a member of loco council
+        """
+        pass
+        #FIXME How to use a user with this rights with openid?
+
+    def test_user_admin_or_owner(self):
+        """
+        test the team update with a user which is owner or admin
+        """
+        pass
+        #FIXME How to use a user with this rights with openid?
+
+
+
+

=== modified file 'loco_directory/teams/urls.py'
--- loco_directory/teams/urls.py	2009-12-04 08:51:48 +0000
+++ loco_directory/teams/urls.py	2009-12-21 09:39:15 +0000
@@ -1,8 +1,8 @@
 from django.conf.urls.defaults import *
 
 urlpatterns = patterns('',
-    url(r'^$', 'teams.views.teams', name='teams'),
+    url(r'^$', 'teams.views.team_list', name='team-list'),
     url(r'reapprovals', 'teams.views.reapprovals'),
-    url(r'(?P<teamedit_slug>[a-zA-Z0-9\-\.\+?]+)/edit$', 'teams.views.team_edit', name='team_editslug'),
-    url(r'(?P<team_slug>[a-zA-Z0-9\-\.\+?]+)$', 'teams.views.team', name='team_slug'),
+    url(r'(?P<team_slug>[a-zA-Z0-9\-\.\+?]+)/edit$', 'teams.views.team_edit', name='team-edit'),
+    url(r'(?P<team_slug>[a-zA-Z0-9\-\.\+?]+)$', 'teams.views.team_detail', name='team-detail'),
 )

=== modified file 'loco_directory/teams/views.py'
--- loco_directory/teams/views.py	2009-12-13 13:58:03 +0000
+++ loco_directory/teams/views.py	2009-12-21 09:39:15 +0000
@@ -4,6 +4,10 @@
 from django.utils.translation import ugettext as _
 from django.core import serializers
 
+from django.contrib.auth.decorators import login_required
+from django.core.urlresolvers import reverse
+from django.http import HttpResponseRedirect
+
 from django.shortcuts import get_object_or_404
 
 from django import http
@@ -18,7 +22,7 @@
 import forms
 
 def reapprovals(request):
-    teams = Team.objects.all_teams().order_by('expires_date')
+    teams = Team.objects.all().order_by('expires_date')
     approved_teams = filter(lambda a: a.approved, teams)
     context = {
         'approved_teams': approved_teams,
@@ -26,8 +30,8 @@
     return common.views.versioned_render('teams/team_reapprovals.html', 
                 context, RequestContext(request))
 
-def teams(request):
-    teams = Team.objects.all_teams()
+def team_list(request):
+    teams = Team.objects.all()
     context = {
         'pageSection': 'teams',
         'teams': utils.trans_sort_object_list(teams, 'name'),
@@ -35,7 +39,7 @@
     return common.views.versioned_render('teams/team_list.html', context,
                   RequestContext(request))
     
-def team(request, team_slug):
+def team_detail(request, team_slug):
     team_object = get_object_or_404(Team, lp_name=team_slug)
     context = {
         'pageSection': 'teams',
@@ -45,57 +49,40 @@
                                          context, RequestContext(request))
 
 
-def team_edit(request, teamedit_slug):
-    try:
-        team_object = Team.objects.get(lp_name=teamedit_slug)
-    except:
-        raise Team.DoesNotExist, "%s" % teamedit_slug
+@login_required
+def team_edit(request, team_slug):
+    #get team
+    team_object = get_object_or_404(Team, lp_name=team_slug)
+    
     context = {
         'pageSection': 'teams',
         'team': team_object,
     }
-#    import logging
-#    logging.basicConfig(level = logging.DEBUG,
-#                        format = '%(asctime)s %(levelname)s %(message)s',
-#                        filename = '/tmp/loco-directory.log',
-#                        filemode = 'a')
-#    logging.debug("%s %s" % (teamedit_slug, request.user))
-    if type(request.user) == AnonymousUser:
-        context["failure_message"] = _("You are not logged in.")
-#        logging.debug("%s %s, not logged in" % (teamedit_slug, request.user))
-        return common.views.versioned_render('teams/team_detail.html', 
-                    context, RequestContext(request))
+    
+    #check if user has needed rights
     is_on_lc = launchpad.is_user_on_loco_council(request.user.username)
     is_contact = launchpad.is_admin_or_owner(request.user.username, team_object)
-#    logging.debug("%s %s - LC: %s, Contact: %s" % (teamedit_slug, request.user, str(is_on_lc), str(is_contact)))
     if not is_on_lc and not is_contact:
-        context["failure_message"] = \
-                _("You are not allowed to make changes to the team details.")
-#        logging.debug("%s %s - LC: %s, Contact: %s, not allowed" % (teamedit_slug, request.user, str(is_on_lc), str(is_contact)))
-        return common.views.versioned_render('teams/team_detail.html', 
-                    context, RequestContext(request))
+        request.user.message_set.create(message=_('You are not allowed to make changes to the team details.'))
+        return HttpResponseRedirect( reverse( 'team-detail', args=[team_slug] ) )
 
     if request.method == "POST":
-#        logging.debug("%s %s - LC: %s, Contact: %s, POSTING" % (teamedit_slug, request.user, str(is_on_lc), str(is_contact)))
         if is_on_lc:
             form = forms.LoCoCouncilForm(data=request.POST, instance=team_object)
         else:
             form = forms.UpdateTeamForm(data=request.POST, instance=team_object)
         if form.is_valid():
             form.save()
-            message = _("Team details updated.")
+            request.user.message_set.create(message=_('Team details updated'))
+            return HttpResponseRedirect( reverse( 'team-detail', args=[team_slug] ) )
         else:
-            message = _("Team details could not be saved.")
-        return common.views.versioned_render('teams/team_update.html', 
-                      {'form': form,
-                       'team':team_object, 
-                       'user':request.user, 
-                       'message':message })
-    if is_on_lc:
-        form = forms.LoCoCouncilForm(instance=team_object)
+            request.user.message_set.create(message=_('Team details could not be saved.'))
     else:
-        form = forms.UpdateTeamForm(instance=team_object)
-#    logging.debug("%s %s - LC: %s, Contact: %s, normal view" % (teamedit_slug, request.user, str(is_on_lc), str(is_contact)))
+        if is_on_lc:
+            form = forms.LoCoCouncilForm(instance=team_object)
+        else:
+            form = forms.UpdateTeamForm(instance=team_object)
+    
     return common.views.versioned_render('teams/team_update.html', 
                     {'form': form, 
                      'team':team_object, 

=== modified file 'loco_directory/templates/500.html'
--- loco_directory/templates/500.html	2009-11-17 17:35:48 +0000
+++ loco_directory/templates/500.html	2009-12-21 09:39:15 +0000
@@ -11,7 +11,7 @@
 
 
 <div class="main">
-  <h2><a href="{% url teams %}">{% trans "You can browse the full list of teams" noop %}</a></h2>
+  <h2><a href="{% url team-list %}">{% trans "You can browse the full list of teams" noop %}</a></h2>
     <p>{% trans "Please make sure the team you are looking for is a member of" noop  %} <a href='https://launchpad.net/~locoteams'>{% trans "The LoCo Teams group in Launchpad" noop %}</a>
     <p>{% trans "If you believe the reason you are here is an bug in the LoCo Directory please " noop  %} <a href='https://bugs.launchpad.net/loco-directory'>{% trans "Report it!" noop %}</a>
     </p>

=== modified file 'loco_directory/templates/about.html'
--- loco_directory/templates/about.html	2009-11-09 11:59:53 +0000
+++ loco_directory/templates/about.html	2009-12-21 09:39:15 +0000
@@ -11,7 +11,7 @@
 
 
 <div class="main">
-  <h2><a href="{% url teams %}">{% trans "What is the LoCo Directory?" noop %}</a></h2>
+  <h2><a href="{% url team-list %}">{% trans "What is the LoCo Directory?" noop %}</a></h2>
     <p>{% trans "The LoCo Directory is a place where you can find information regarding LoCo Teams. It was originally developed by <a href='https://launchpad.net/~nixternal'>Richard Johnson</a> and further development has been made possible by the <a href='https://launchpad.net/~loco-directory-dev'>The LoCo Directory Developers</a>." noop  %}
 {% trans "The LoCo Directory is made available under the <a href='http://www.gnu.org/licenses/gpl-3.0-standalone.html'>GNU GPL v3</a>." noop %}
     </p>
@@ -20,7 +20,7 @@
 <div id="container2">
   <div id="left">
 
-<h2><a href="{% url teams %}">{% trans "The Directory Data" noop %}</a></h2>
+<h2><a href="{% url team-list %}">{% trans "The Directory Data" noop %}</a></h2>
 <p>{% trans "The LoCo Directory comes from two sources, <a href='https://launchpad.net/'>Launchpad</a> and its <a href='https://help.launchpad.net/API/launchpadlib'>APIs</a>. The data that the LoCo Directory gathers is also made available as  <a href='/data/xml'>XML</a> format to interoperate with other web apps and for possible mashups." noop  %}
 {% trans "The LoCo Directory is made available under the <a href='http://www.gnu.org/licenses/gpl-3.0-standalone.html'>GNU GPL v3</a> and the Code is available in <a href='https://code.launchpad.net/loco-directory'>Launchpad Code Hosting</a>." noop %}
 </p>

=== modified file 'loco_directory/templates/index.html'
--- loco_directory/templates/index.html	2009-04-03 08:18:07 +0000
+++ loco_directory/templates/index.html	2009-12-21 09:39:15 +0000
@@ -11,7 +11,7 @@
 
 <table><tr><td width="50%" valign="top">
 
-<h2><a href="{% url teams %}">{% trans "Teams" %}</a></h2>
+<h2><a href="{% url team-list %}">{% trans "Teams" %}</a></h2>
 <p>{% trans "List of all Ubuntu LoCo Teams, with full information such as contact, web pages, mailing list, and more." %}
 {% trans "If there are no teams in your country/region/area, you will be able to add a team in the near future." %}
 </p>

=== modified file 'loco_directory/templates/teams/team_detail.html'
--- loco_directory/templates/teams/team_detail.html	2009-12-10 11:31:14 +0000
+++ loco_directory/templates/teams/team_detail.html	2009-12-21 09:39:15 +0000
@@ -5,13 +5,10 @@
 
 {% block content %}
 <div class="team_detail">
-{% if failure_message %}
-<p>
-  <b><font color="red">{{ failure_message }}</font></b>
-</p>
-{% endif %}
-<p><b><a href="/teams">{% trans "Go back to Team list." %}</a></b></p>
-<p><b><a href="/teams/{{ team.lp_name }}/edit">{% trans "Edit team details." %}</a></b></p>
+		<p><b><a href="{% url team-list %}">{% trans "Go back to Team list." %}</a></b></p>
+		{% if user.is_authenticated %}
+				<p><b><a href="{% url team-edit team.lp_name %}">{% trans "Edit team details." %}</a></b></p>
+		{% endif %}
 <h2><a href="https://launchpad.net/~{{ team.lp_name }}">{% trans team.name %}</a></h2>
 <table cellpadding="0" cellspacing="0">
 <tr><th colspan="2">{% trans "Location Information" %}</th></tr>

=== modified file 'loco_directory/templates/teams/team_list.html'
--- loco_directory/templates/teams/team_list.html	2009-06-24 09:29:09 +0000
+++ loco_directory/templates/teams/team_list.html	2009-12-21 09:39:15 +0000
@@ -22,9 +22,9 @@
 <ul>
 {% for team in column %}
 {% if team.approved %}
-<li class="approved"><a href="{{ team.get_absolute_url }}">{{ team.translated_name }}</a></li>
+<li class="approved"><a href="{% url team-detail team.lp_name %}">{{ team.translated_name }}</a></li>
 {% else %}
-<li class="unapproved"><a href="{{ team.get_absolute_url }}">{{ team.translated_name }}</a></li>
+<li class="unapproved"><a href="{% url team-detail team.lp_name %}">{{ team.translated_name }}</a></li>
 {% endif %}
 {% endfor %}
 </ul>

=== modified file 'loco_directory/templates/teams/team_reapprovals.html'
--- loco_directory/templates/teams/team_reapprovals.html	2009-06-24 09:29:09 +0000
+++ loco_directory/templates/teams/team_reapprovals.html	2009-12-21 09:39:15 +0000
@@ -14,7 +14,7 @@
 <td width="100%" valign="top" class="col2_container">
 <ul>
 {% for team in approved_teams %}
-<li class="approved"><a href="{{ team.get_absolute_url }}">{{ team.name }}</a>: {{ team.expires_date }}</li>
+<li class="approved"><a href="{% url team-detail team.lp_name %}">{{ team.name }}</a>: {{ team.expires_date }}</li>
 {% endfor %}
 </ul>
 </td>

=== modified file 'loco_directory/templates/teams/team_update.html'
--- loco_directory/templates/teams/team_update.html	2009-12-10 09:12:23 +0000
+++ loco_directory/templates/teams/team_update.html	2009-12-21 09:39:15 +0000
@@ -7,14 +7,6 @@
 
 <p><b><a href="/teams">{% trans "Go back to Team list." %}</a></b></p>
 <p><b><a href="/teams/{{ team.lp_name }}">{% trans "Go back to team details." %}</a></b></p>
-<div id="form">
-    {% if message %}
-	<div class="box">
-	  {{ message }}
-	  <img src="/media/img/delete.png" alt="delete" class="delete" />
-        </div>
-    {% endif %} 
-  </div>
   <div id="form">
     {% if form.errors %}
         <p style="color: red;">