Merge lp:~dholbach/harvest/581724 into lp:harvest

[Daniel Holbach]

[James Westby (james-w)]

Looks good to me.

Do django templates automatically call methods in {{{}}} blocks?

Thanks,

James

review: Approve

[Daniel Holbach (dholbach)]

Dylan just asked me to use the {%url <...> %} templatetag instead, so I'll have a look into using that instead.

Thanks a bunch for the review!

review: Needs Fixing

[Daniel Holbach (dholbach)]

This is weird, my fresh checkout of the branch does not contain any "+<<<<<<< TREE".

[Dylan McCall (dylanmccall)]

Try rebasing it. Maybe it's Launchpad being funny about potential merge conflicts :)

Thanks for doing that with the url templatetag. It helps to separate the front end from the rest, which should serve us well in the future :)
It looks right to me!

I am not a huge fan of line 124:

  (<a href="{% url opportunity_detail opportunity.id %}edit?next={{request.get_full_path}}">edit</a>)

The hard-coded “edit” added to the end is asking for trouble. Would be nice to have a separate opportunity_edit URL. I think there is a templatetag somewhere to append GET parameters a bit more elegantly, too, although that isn't so much of an issue…

The same was there before, though, so we can just put it on the list of little things that don't really _need_ to be fixed, but would be satisfying to do :)

[Daniel Holbach (dholbach)]

Ok, so I fixed the URL, but it doesn't get better than this:

    {% url opportunity_edit opportunity.id %}/?next={{request.get_full_path}}

We need 'next' as an argument as part of the URL for some reason I forgot, but at least I'm not munging the opportunity_edit URL together now, but use the real one. :-)

If somebody has an idea how to make this even better, please let me know. :)

[Daniel Holbach (dholbach)]

Can this be landed? :)

[James Westby (james-w)]

8 - def __unicode__(self):
9 - return self.name

Did you mean to delete that?

Otherwise this looks good to me.

Thanks,

James

review: Approve

[Daniel Holbach (dholbach)]

Thanks for the review James - I re-added the __unicode__ function - that was slightly muppet-esque and I fixed it. :-)

I just saw that you approved the last proposal, so I'll go ahead and merge it.

=== modified file 'harvest/opportunities/models.py'
--- harvest/opportunities/models.py	2010-06-04 14:55:30 +0000
+++ harvest/opportunities/models.py	2010-06-15 08:49:24 +0000
@@ -20,10 +20,6 @@
     def __unicode__(self):
         return self.name
 
-    @models.permalink
-    def get_absolute_url(self):
-        return ('packagesets', [self.name])
-
 
 class SourcePackage(models.Model):
     name = models.SlugField(_("Name"), max_length=70)
@@ -35,10 +31,6 @@
     def __unicode__(self):
         return self.name
 
-    @models.permalink
-    def get_absolute_url(self):
-        return ('sourcepackage_detail', [self.name])
-
     @property
     def valid_opportunities(self):
         return self.opportunity_set.filter(valid=True)
@@ -73,10 +65,6 @@
     def __unicode__(self):
         return self.name
 
-    @models.permalink
-    def get_absolute_url(self):
-        return ('opportunitylist_detail', [self.name])
-
     @property
     def opportunity_urgency(self):
         if self.featured:

=== modified file 'harvest/opportunities/templates/opportunities/opportunities_by_package.html'
--- harvest/opportunities/templates/opportunities/opportunities_by_package.html	2010-02-22 15:43:51 +0000
+++ harvest/opportunities/templates/opportunities/opportunities_by_package.html	2010-06-15 08:49:24 +0000
@@ -13,7 +13,7 @@
   {% for source in sources.object_list %}
     <li>
       <div class="package_name threshold{{ source.opportunity_class }}">
-        <span class="name">{{ source.name }}</span>
+        <span class="name"><a href="{% url sourcepackage_detail source.name %}">{{ source.name }}</a></span>
         <span class="count">
             ({{ source.valid_opportunities.count }} issues)
         </span>

=== modified file 'harvest/opportunities/views.py'
--- harvest/opportunities/views.py	2010-06-03 11:16:57 +0000
+++ harvest/opportunities/views.py	2010-06-15 08:49:24 +0000
@@ -36,7 +36,7 @@
 def opportunity_list(request):
     return list_detail.object_list(
         request,
-        queryset = models.Opportunity.objects.all(),
+        queryset = models.Opportunity.objects.filter(last_updated=opportunitylist__last_updated),
         paginate_by = 500,
         template_object_name = 'opportunity',
     )
@@ -44,7 +44,7 @@
 def opportunity_detail(request, opportunity_id):
     return list_detail.object_detail(
         request,
-        queryset = models.Opportunity.objects.all(),
+        queryset = models.Opportunity.objects.filter(last_updated=opportunitylist__last_updated),
         object_id = opportunity_id,
         template_object_name = 'opportunity',
     )
@@ -121,7 +121,7 @@
     )
 
 def opportunities_by_type(request):
-    types_list = models.OpportunityList.objects.all()
+    types_list = models.OpportunityList.objects.filter(active=True)
     paginator = Paginator(types_list, 50)
 
     # Make sure page request is an int. If not, deliver first page.

=== modified file 'harvest/templates/opportunities/opportunity_detail.inc.html'
--- harvest/templates/opportunities/opportunity_detail.inc.html	2010-03-08 16:43:38 +0000
+++ harvest/templates/opportunities/opportunity_detail.inc.html	2010-06-15 08:49:24 +0000
@@ -3,19 +3,19 @@
                 <a href="{{opportunity.url}}">{{ opportunity.description }}</a>
 		{% if grouping %}
 		    {% ifequal grouping "type" %}
-                        ({{ opportunity.sourcepackage }})
+                        <a href="{% url sourcepackage_detail opportunity.sourcepackage.name %}">({{ opportunity.sourcepackage }})</a>
 		    {% endifequal %}
 		    {% ifequal grouping "package" %}
                         ({{ opportunity.opportunitylist }})
                     {% endifequal %}
 		{% else %}
-                    ({{ opportunity.sourcepackage }} - {{ opportunity.opportunitylist }})
+                    (<a href="{% url sourcepackage_detail opportunity.sourcepackage.name %}">({{ opportunity.sourcepackage }})</a> - {{ opportunity.opportunitylist }})
 		{% endif %}
 		{% if opportunity.experience %}
 		    <span class="experience{{ opportunity.experience }}"></span>
 		{% endif %}
 		{% if user.is_authenticated %}
-		    (<a href="{{opportunity.get_absolute_url}}edit?next={{request.get_full_path}}">edit</a>)
+		    (<a href="{% url opportunity_edit opportunity.id %}?next={{request.get_full_path}}">edit</a>)
                 {% endif %}
             </span>
 	</li>

=== modified file 'harvest/templates/opportunities/opportunity_index.html'
--- harvest/templates/opportunities/opportunity_index.html	2010-02-22 15:43:51 +0000
+++ harvest/templates/opportunities/opportunity_index.html	2010-06-15 08:49:24 +0000
@@ -11,7 +11,7 @@
 {% if sources.object_list %}
 <ul>
 {% for source in sources.object_list %}
-<li>{{ source.name }}
+<li><a href="{% url sourcepackage_detail source.name %}">{{ source.name }}</a>
 <ul>
 {% for opportunity in source.valid_opportunites %}
     {% include "opportunities/opportunity_detail.inc.html" %}

=== modified file 'harvest/templates/opportunities/opportunitylist_list.html'
--- harvest/templates/opportunities/opportunitylist_list.html	2009-09-13 16:53:16 +0000
+++ harvest/templates/opportunities/opportunitylist_list.html	2010-06-15 08:49:24 +0000
@@ -7,7 +7,7 @@
 {% block content %}
     <h2>Opportunity Lists</h2>
     {% for opportunity_list in object_list %}
-        <h3><a href="{{ opportunity_list.get_absolute_url }}">{{ opportunity_list.name }}</a> ({{ opportunity_list.opportunity__count }} issue{{ opportunity_list.opportunity__count|pluralize }})</h3>
+        <h3><a href="{% url opportunitylist_detail opportunity_list %}">{{ opportunity_list.name }}</a> ({{ opportunity_list.opportunity__count }} issue{{ opportunity_list.opportunity__count|pluralize }})</h3>
         <p>{{ opportunity_list.description }}</p>
     {% endfor %}
 {% endblock %}

=== modified file 'harvest/templates/opportunities/sourcepackage_list.html'
--- harvest/templates/opportunities/sourcepackage_list.html	2009-09-13 16:53:16 +0000
+++ harvest/templates/opportunities/sourcepackage_list.html	2010-06-15 08:49:24 +0000
@@ -8,7 +8,7 @@
     <h2>Source Packages</h2>
     <ul>
         {% for source_package in object_list %}
-            <li><a href="{{ source_package.get_absolute_url }}">{{ source_package.name }}</a> {{ source_package.opportunity__count }} issue{{ source_package.opportunity__count|pluralize }}</li>
+            <li><a href="{% url sourcepackage_detail source_package %}">{{ source_package.name }}</a> {{ source_package.opportunity__count }} issue{{ source_package.opportunity__count|pluralize }}</li>
         {% endfor %}
     </ul>
 {% endblock %}