Merge lp:~dylanmccall/harvest/opportunity-edit into lp:harvest

[Dylan McCall]

Improved the opportunity edit form and the style for some surrounding elements. Implemented the new Notes system (which is like comments, but shorter and simpler with a 250 character limit).

Screenshot: http://people.ubuntu.com/~dylanmccall/harvest/media/gsoc-week12/opportunity-edit.png

This branch does not address how the form is opened or where it goes after submitting, which is still a problem when running with Javascript.

[lp:~dylanmccall/harvest/opportunity-edit]

243. By Dylan McCall on 2010-08-13

Improved formatting for form errors and #messages.

Fixed some inconsistency with padding and margins around elements.

244. By Dylan McCall on 2010-08-13

Fixed wrapping for .opportunity-details.edit > form > ul.opportunity-switches

[Dylan McCall (dylanmccall)]

Oh, I should mention: Note adds a new table to the database, and the Comment field in Opportunity is removed (replaced with note_set via the OneToMany relationship).

You'll need to run ./manage.py syncdb to get that all working right. It shouldn't cause any problems given that the comment field hasn't been used for anything so far.

[Daniel Holbach (dholbach)]

Good work. I like your code changes, but there's a few small glitches which we can address through follow-up bugs.

review: Approve

[Daniel Holbach (dholbach)]

Nevermind, can't reproduce the issue I had with duplicated opportunity entries.

=== modified file 'EXTERNALS'
--- EXTERNALS	2010-07-14 15:34:17 +0000
+++ EXTERNALS	2010-08-13 02:04:41 +0000
@@ -10,6 +10,10 @@
 	By "metajack"
 	<http://djangosnippets.org/snippets/797/>
 
+./harvest/common/templatetags/humanize_timediff.py
+	By "supsupmo"
+	<http://djangosnippets.org/snippets/412/>
+
 ./harvest/media/css/reset.css
 	By Eric Meyer
 	<http://meyerweb.com/eric/tools/css/reset/>
 
=== added file 'harvest/common/templatetags/humanize_timediff.py'
--- harvest/common/templatetags/humanize_timediff.py	1970-01-01 00:00:00 +0000
+++ harvest/common/templatetags/humanize_timediff.py	2010-08-13 02:04:41 +0000
@@ -0,0 +1,51 @@
+#Template filter to provide a string similar to the built in timesince
+#filter, but slightly fuzzier.
+#From <http://djangosnippets.org/snippets/412/> with localization added
+
+from django.utils.translation import ungettext
+from django import template
+
+register = template.Library()
+
[e-mail]
+def humanize_timediff(timestamp = None):
+    """
+    Returns a humanized string representing time difference
+    between now() and the input timestamp.
+    
+    The output rounds up to days, hours, minutes, or seconds.
+    4 days 5 hours returns '4 days'
+    0 days 4 hours 3 minutes returns '4 hours', etc...
+    """
+    import datetime
+    
+    timeDiff = datetime.datetime.now() - timestamp
+    days = timeDiff.days
+    hours = timeDiff.seconds/3600
+    minutes = timeDiff.seconds%3600/60
+    seconds = timeDiff.seconds%3600%60
+    
+    str = ""
+    tStr = ""
+    if days > 0:
+        str = ungettext('%(days)d day', '%(days)d days', days) % {
+            'days' : days
+        }
+        return str
+    elif hours > 0:
+        str = ungettext('%(hours)d hour', '%(hours)d hours', hours) % {
+            'hours' : hours
+        }
+        return str
+    elif minutes > 0:
+        str = ungettext('%(minutes)d minute', '%(minutes)d minutes', minutes) % {
+            'minutes' : minutes
+        }
+        return str
+    elif seconds > 0:
+        str = ungettext('%(seconds)d second', '%(seconds)d seconds', seconds) % {
+            'seconds' : seconds
+        }
+        return str
+    else:
+        return None
 
=== removed file 'harvest/common/templatetags/list_to_columns.py'
--- harvest/common/templatetags/list_to_columns.py	2009-07-08 11:31:32 +0000
+++ harvest/common/templatetags/list_to_columns.py	1970-01-01 00:00:00 +0000
@@ -1,30 +0,0 @@
-from django import template
-
-register = template.Library()
-
-class SplitListNode(template.Node):
-    def __init__(self, list, cols, new_list):
-        self.list = list
-        self.cols = cols
-        self.new_list = new_list
-
-    def split_seq(self, list, cols=2):
-        start = 0
-        for i in xrange(cols):
-            stop = start + len(list[i::cols])
-            yield list[start:stop]
-            start = stop
-
-    def render(self, context):
-        context[self.new_list] = self.split_seq(context[self.list],
-            int(self.cols))
-        return ''
-
-def list_to_columns(parser, token):
-    bits = token.contents.split()
-    if len(bits) != 5:
-        raise template.TemplateSyntaxError, "list_to_columns list as new_list 2"
-    if bits[2] != 'as':
-        raise template.TemplateSyntaxError, "second argument to the list_to_columns tag must be 'as'"
-    return SplitListNode(bits[1], bits[4], bits[3])
-list_to_columns = register.tag(list_to_columns)
 
=== modified file 'harvest/media/css/style.css'
--- harvest/media/css/style.css	2010-08-05 17:20:01 +0000
+++ harvest/media/css/style.css	2010-08-13 02:04:41 +0000
@@ -43,6 +43,17 @@
 	color:rgb(180,180,180);
 }
 
+form label {
+	font-weight:bold;
+}
+form .error input, form .error textarea, form .error input#new_note  {
+	border:solid 2px rgb(140,0,0);
+}
+form ul.errorlist {
+	font-size:smaller;
+	color:rgb(140,0,0);
+}
+
 
 
 #container {
@@ -59,34 +70,33 @@
 	display:block;
 	width:100%;
 	min-height:80px;
-	
 	padding-top:12px;
-	padding-bottom:12px;
 }
 #header a {
 	color:rgb(0,0,0);
 }
-#header #pagetitle {
+#header #sitetitle {
 	display:inline;
 	position:static;
-	margin-left:80px;
+	margin-left:40px;
 	margin-right:40px;
+	margin-bottom:10px;
 	
 	text-transform:lowercase;
 	color:rgb(0,0,0);
 	font-family:'Molengo', 'Bitstream Vera Sans', 'DejaVu Sans', sans-serif;
 }
-#header #pagetitle #sitelogo {
+#header #sitetitle #sitelogo {
 	width:auto; /* line up with #sitename font-size */
 	height:36px;
 }
-#header #pagetitle #sitename {
+#header #sitetitle #sitename {
 	display:inline;
 	position:static;
 	
 	font-size:36px;
 }
-#header #pagetitle #releasename {
+#header #sitetitle #releasename {
 	display:inline;
 	position:static;
 	
@@ -112,6 +122,30 @@
 	padding-bottom:140px;
 }
 
+#content > #messages {
+	display:block;
+	margin:10px 0px;
+	padding:5px 10px;
+	font-size:larger;
+	background-color:rgb(242,151,93);
+	color:rgb(255,255,255);
+}
+
+#content > .pagetitle {
+	margin:10px 0px;
+	max-width:30em;
+	padding:0px 10px;
+	letter-spacing:-1px;
+	font-size:18px;
+	line-height:1em;
+}
+
+#content > .main {
+	margin:10px 0px;
+	padding:0px 20px;
+	max-width:60em;
+}
+
 
 
 #filters {
@@ -120,7 +154,7 @@
 	float:left;
 	min-width:160px;
 	
-	padding:0px 20px 20px 20px;
+	padding:0px 20px;
 	font-size:12px;
 	line-height:1.4em;
 	color:rgb(51,51,51);
@@ -207,9 +241,9 @@
 #filters .editfilter input {
 	width:8em;
 	border:none;
+	border-bottom:1px solid rgb(180,180,180);
+	
 	padding:0px 2px 0px 2px;
-	border-bottom:1px solid rgb(180,180,180);
-	
 	background-color:inherit;
 	color:inherit;
 	font:inherit;
@@ -265,48 +299,43 @@
 	-moz-border-radius:3px 3px 0px 0px;
 	border-radius:3px 3px 0px 0px; /* lines up with li.sourcepackage's border */
 }
-.sourcepackage > a.sourcepackage-header {
+a.sourcepackage-header {
 	color:inherit;
 	text-decoration:none;
 }
-.sourcepackage > .sourcepackage-header > .sourcepackage-name {
+.sourcepackage-header > .sourcepackage-name {
 	display:inline;
 	margin-right:10px;
 	color:rgb(0,0,0);
 	font-size:16px;
 }
-.sourcepackage > .sourcepackage-header > .status {
+.sourcepackage-header > .status {
 	display:none; /* harvest.js will override this where necessary */
 }
-.sourcepackage > .sourcepackage-header > .status img {
+.sourcepackage-header > .status img {
 	width:16px;
 	height:16px;
 }
-.sourcepackage > .sourcepackage-header > .sourcepackage-summary {
+.sourcepackage-header > .sourcepackage-summary {
 	display:inline;
 	float:right;
 	text-align:right;
 	font-size:10px;
 }
 /* prelights */
-.sourcepackage > a.sourcepackage-header:hover,
-.sourcepackage > a.sourcepackage-header:focus {
-	/* use the opportunity count or experience measure here */
+a.sourcepackage-header:hover,
+a.sourcepackage-header:focus {
 	background-color:rgb(240,255,243);
 }
-.sourcepackage > a.sourcepackage-header:hover .sourcepackage-name,
-.sourcepackage > a.sourcepackage-header:focus .sourcepackage-name {
+a.sourcepackage-header:hover .sourcepackage-name,
+a.sourcepackage-header:focus .sourcepackage-name {
 	text-decoration:underline;
 }
 
-.sourcepackage > .sourcepackage-details {
+.sourcepackage-details {
 	display:block;
-	padding-top:20px;
 	/* inner padding should be left:20px, right:20px and bottom:5px */
 }
-li.sourcepackage > .sourcepackage-details {
-	padding-top:0px;
-}
 
 /* collapsed state. (expanded is the default) */
 li.sourcepackage.collapsed {
@@ -334,7 +363,7 @@
 }
 
 
-.sourcepackage-details > .opportunity-list {
+li.sourcepackage > .sourcepackage-details > .opportunity-list {
 	padding-left:20px;
 	margin-bottom:5px;
 }
@@ -394,17 +423,71 @@
 	font-size:smaller;
 }
 
-.opportunity > .opportunity-details {
+li.opportunity > .opportunity-details {
 	display:block;
 	margin-left:10px;
 }
-.opportunity-details > .opportunity-status {
+li.opportunity > .opportunity-details.edit {
+	background-color:rgb(255,255,240);
+}
+
+.opportunity-notes {
 	display:block;
-	max-width:30em;
+	width:100%;
+	max-width:35em;
 	border-left:dashed 1px rgb(180,180,180);
-	padding-left:5px;
+	padding:0px 5px 0px 5px;
 	font-size:12px;
 }
+.opportunity-notes input#new_note {
+	width:100%;
+	border:none;
+	border-bottom:1px solid rgb(180,180,180);
+	
+	padding:2px 5px 2px 5px;
+	background-color:inherit;
+	color:inherit;
+	font:inherit;
+}
+.opportunity-notes > ul {
+	margin:0.5em 0;
+	max-height:10em;
+}
+.opportunity-notes > ul > li {
+	line-height:1.2em;
+	margin-bottom:0.5em;
+}
+.opportunity-notes > ul > li > .signature {
+	margin-left:1em;
+	vertical-align:sub;
+	color:rgb(180,180,180);
+	font-style:italic;
+	font-size:smaller;
+}
+
+
+
+.opportunity-details.edit > form > div.opportunity-notes {
+	float:left;
+	margin-right:4em;
+	margin-bottom:2em;
+}
+
+.opportunity-details.edit > form > ul.opportunity-switches {
+	float:left;
+}
+.opportunity-details.edit > form > ul.opportunity-switches > li {
+	white-space:nowrap;
+	margin-bottom:0.5em;
+}
+.opportunity-details.edit > form > ul.opportunity-switches > li.separate-top {
+	margin-bottom:2em;
+}
+
+.opportunity-details.edit > form > .actions {
+	clear:both;
+	padding-top:1em;
+}
 
 
 
 
=== modified file 'harvest/media/js/harvest.js'
--- harvest/media/js/harvest.js	2010-07-21 20:59:34 +0000
+++ harvest/media/js/harvest.js	2010-08-13 02:04:41 +0000
@@ -52,6 +52,10 @@
 var harvest = new function () {
 
 /* Globals and constants */
+var XHR_ROOT = '/opportunities/xhr/'; /* TODO: would be nice to get this from Django */
+var RESULTS_URL = XHR_ROOT + 'results/'
+var OPPORTUNITIES_URL = XHR_ROOT + 'opportunity/';
+
 var MEDIA_PATH = '/media/' /* we should get this from Django somehow */
 var MEDIA = { 'pkg'     :  { 'loading' : MEDIA_PATH+'img/pkg-status-loading.gif' },
               'results' :  { 'waiting' : MEDIA_PATH+'img/results-status-waiting.gif',
@@ -312,13 +316,13 @@
 }
 
 
-function Package (dom_node, details_url, opps_query, expanded_cb, collapsed_cb) {
+function Package (dom_node, opps_query, expanded_cb, collapsed_cb) {
 	/* Created for each package inside the #results element */
 	
-	/* gtksourceview gives an error box around "package", so we'll have to forego the convention */
+	/* gtksourceview highlights "package" as an error, so we'll have to forego the convention */
 	var pkg = this;
 	
-	this.id = $(dom_node).attr('data-results-packageid');
+	this.id = $(dom_node).attr('data-package-id');
 	this.details = $(dom_node).children('.sourcepackage-details');
 	
 	this.loading_xhr = null;
@@ -370,7 +374,7 @@
 			
 			this.loading_xhr = $.ajax({
 				type: "GET",
-				url: details_url + this.id, dataType: 'html',
+				url: RESULTS_URL + this.id, dataType: 'html',
 				data: opps_query,
 				complete: function (xhr, status) {
 						pkg.hide_status('loading');
@@ -452,7 +456,6 @@
 	this.future_query = {};
 	
 	this.container = $(dom_node);
-	this.query_url = $(dom_node).attr('data-results-url');
 	this.output = $(dom_node).children('#results');
 	this.status_bubble = $(dom_node).children('#results-status');
 	
@@ -488,10 +491,9 @@
 		
 		results_packages.each(function () {
 			var dom_node = $(this);
-			var details_url = results.query_url + '/';
 			var opps_query = results.current_query; /* would be nice to only send properties starting with opp: */
 			
-			var pkg = new Package(dom_node, details_url, opps_query,
+			var pkg = new Package(dom_node, opps_query,
 			                      pkg_expanded_cb, pkg_collapsed_cb);
 			results.packages[pkg.id] = pkg;
 			
@@ -545,7 +547,7 @@
 		
 		this.loading_xhr = $.ajax({
 			type: "GET",
-			url: this.query_url, dataType: 'html',
+			url: RESULTS_URL, dataType: 'html',
 			data: load_query,
 			complete: function (xhr, status) {
 					results.hide_status('loading');
 
=== modified file 'harvest/opportunities/admin.py'
--- harvest/opportunities/admin.py	2009-08-30 14:53:53 +0000
+++ harvest/opportunities/admin.py	2010-08-13 02:04:41 +0000
@@ -1,9 +1,15 @@
 from django.contrib import admin
-from opportunities.models import Opportunity, OpportunityList, SourcePackage
+from opportunities.models import Opportunity, Note, OpportunityList, SourcePackage
+
+class NoteInline(admin.TabularInline):
+    model = Note
+    extra = 1
+    raw_id_fields = ('author', )
 
 class OpportunityAdmin(admin.ModelAdmin):
-    list_display = ('description', 'sourcepackage', 'opportunitylist', 'last_updated')
+    list_display = ('description', 'sourcepackage', 'opportunitylist', 'last_updated', 'note_set')
     list_filter = ('opportunitylist', 'last_updated')
+    inlines = [NoteInline]
 
 class OpportunityListAdmin(admin.ModelAdmin):
     list_display = ('name', 'description', 'last_updated', 'active')
 
=== modified file 'harvest/opportunities/forms.py'
--- harvest/opportunities/forms.py	2010-03-08 16:53:44 +0000
+++ harvest/opportunities/forms.py	2010-08-13 02:04:41 +0000
@@ -1,9 +1,13 @@
 from django import forms
-
 from models import Opportunity
+from django.utils.translation import ugettext as _
 
 class OpportunityForm(forms.ModelForm):
+    new_note = forms.CharField(label=_("Enter a new note here"),
+                               required=False,
+                               max_length=250) #from models.Note.text
+    
     class Meta:
         model = Opportunity
-        exclude = ('description', 'url', 'last_updated', 'since', 'sourcepackage', 'opportunitylist', 'valid')
+        fields = ('experience', 'applied', 'reviewed')
 
 
=== modified file 'harvest/opportunities/models.py'
--- harvest/opportunities/models.py	2010-08-05 17:20:01 +0000
+++ harvest/opportunities/models.py	2010-08-13 02:04:41 +0000
@@ -8,10 +8,10 @@
 PACKAGE_RED_THRESHOLD = 20
 
 EXPERIENCE_CHOICES = (
-    (0, '---'),
-    (1, 'Easy'),
-    (2, 'Medium'),
-    (3, 'Hard'),
+    (0, ""),
+    (1, _("Easy")),
+    (2, _("Medium")),
+    (3, _("Hard")),
 )
 
 class PackageSet(models.Model):
@@ -83,9 +83,8 @@
     applied = models.BooleanField(_("Applied"), default=False, blank=True)
     sourcepackage = models.ForeignKey(SourcePackage)
     opportunitylist = models.ForeignKey(OpportunityList)
-    comment = models.TextField(_("Comment"), blank=True)
     valid = models.BooleanField(_("Valid"), default=True)
-    experience = models.IntegerField(_("Required Experience"), choices=EXPERIENCE_CHOICES, default=0,
+    experience = models.IntegerField(_("Difficulty"), choices=EXPERIENCE_CHOICES, default=0,
                                      help_text=_("Level of experience required for this specific opportunity."))
 
     class Meta:
@@ -108,6 +107,20 @@
             summary_list.append(_("Invalid"))
         return summary_list
 
+
+class Note(models.Model):
+    opportunity = models.ForeignKey(Opportunity)
+    date = models.DateTimeField(auto_now_add=True)
+    author = models.ForeignKey(User)
+    text = models.CharField(max_length=250)
+    
+    def __unicode__(self):
+        text = self.text
+        if len(text) > 40:
+            text = text[:40]+u"\u2026"
+        return '%s: %s' % (self.author, text)
+
+
 class ActionLogEntry(models.Model):
     timestamp = models.DateTimeField(_("Timestamp"))
     who = models.ForeignKey(User)
 
=== modified file 'harvest/opportunities/urls.py'
--- harvest/opportunities/urls.py	2010-07-30 21:45:23 +0000
+++ harvest/opportunities/urls.py	2010-08-13 02:04:41 +0000
@@ -1,12 +1,28 @@
 from django.conf.urls.defaults import *
 
 urlpatterns = patterns('',
-    url(r'^opportunity/(?P<opportunity_id>[\d]+)/edit$', 'opportunities.views.opportunity_edit', name='opportunity_edit'),
-    
-    url(r'^filter$', 'opportunities.views.opportunities_filter', name='opportunities_filter'),
-    
-    url(r'^package/(?P<package_name>.+)', 'opportunities.views.single_package', name='single_package'),
-    
-    url(r'^xhr/results$', 'opportunities.views.opportunities_xhr_filter_results', name='opportunities_xhr_filter_results'),
-    url(r'^xhr/results/(?P<package_id>[\d]+)$', 'opportunities.views.opportunities_xhr_package_details', name='opportunities_xhr_package_details'),
+    url(r'^$',
+        'opportunities.views.filter',
+        name='filter'),
+    
+    url(r'^package/(?P<package_name>.+)/$',
+        'opportunities.views.single_package',
+        name='single_package'),
+    
+    url(r'^opportunity/(?P<opportunity_id>[\d]+)/edit/$',
+        'opportunities.views.opportunity_edit',
+        name='opportunity_edit'),
+    
+    
+    url(r'^xhr/results/$',
+        'opportunities.views.xhr_filter_results'),
+    
+    url(r'^xhr/results/(?P<package_id>[\d]+)/$',
+        'opportunities.views.xhr_package_details'),
+    
+    url(r'xhr/opportunity/(?P<opportunity_id>[\d]+)/$',
+        'opportunities.views.xhr_opportunity_li'),
+    
+    url(r'^xhr/opportunity/(?P<opportunity_id>[\d]+)/edit/$',
+        'opportunities.views.xhr_opportunity_edit'),
 )
 
=== modified file 'harvest/opportunities/views.py'
--- harvest/opportunities/views.py	2010-07-30 21:45:23 +0000
+++ harvest/opportunities/views.py	2010-08-13 02:04:41 +0000
@@ -13,11 +13,67 @@
 import models
 import forms
 
+from models import Opportunity, Note # for form processing
+
 from filters import HarvestFilters
 from wrappers import PackageWrapper, PackageListWrapper
 
+def _create_packages_list(request, filters_pkg, filters_opp):
+    # XXX: rockstar: Eep! We shouldn't be storing the None as a string.  We
+    # should re-think this model relationship.
+    #sourcepackages_list = models.SourcePackage.objects.exclude(name='None')
+    
+    sourcepackages_list = models.SourcePackage.objects.distinct()
+    sourcepackages_list = filters_pkg.process_queryset(sourcepackages_list)
+    
+    #opportunities_list is filtered right away to only check opportunities belonging to selected packages
+    opportunities_list = models.Opportunity.objects.distinct().filter(sourcepackage__in=sourcepackages_list)
+    opportunities_list = filters_opp.process_queryset(opportunities_list)
+    
+    #TODO: need to filter out opportunities with valid=False again
+    #TODO: would it be more efficient to group opportunities by their sourcepackages first, then run filters_opp.process_queryset() for each of those groups?
+    
+    pkg_list_wrapper = PackageListWrapper(request, sourcepackages_list, opportunities_list)
+    
+    return pkg_list_wrapper
+
+
+
+def filter(request):
+    filters = HarvestFilters()
+    filters.update_from_http(request)
+    filters_pkg = filters.find('pkg')
+    filters_opp = filters.find('opp')
+    
+    pkg_list_wrapper = _create_packages_list(request, filters_pkg, filters_opp) 
+    
+    context = {
+        'packages_list': pkg_list_wrapper,
+        'filters_pkg' : filters_pkg,
+        'filters_opp' : filters_opp
+    }
+
+    return render(
+        'opportunities/filter.html',
+        context,
+        context_instance=RequestContext(request))
+
+def single_package(request, package_name):
+    package = get_object_or_404(models.SourcePackage, name=package_name)
+    
+    package_wrapper = PackageWrapper(request, package, visible_opportunities = package.opportunity_set)
+    
+    context = {
+        'package': package_wrapper
+    }
+    
+    return render(
+        'opportunities/single_package.html',
+        context,
+        context_instance=RequestContext(request))
+
 @login_required
-def opportunity_edit(request, opportunity_id):
+def opportunity_edit(request, opportunity_id, template='opportunities/opportunity_edit.html'):
     opportunity = get_object_or_404(models.Opportunity, id=opportunity_id)
     if request.method == "POST":
         form = forms.OpportunityForm(data=request.POST, instance=opportunity)
@@ -32,68 +88,41 @@
                 models.log_action(request.user, 
                                   action="changed experience to: %s" % form.cleaned_data["experience"],
                                   opportunity=opportunity)
-            if form.cleaned_data["comment"] != opportunity.comment:
-                if len(form.cleaned_data["comment"]) > 178:
-                    action = "changed comment to: '%s'" % (form.cleaned_data["comment"][:177]+u"…")
-                else:
-                    action = "changed comment to: '%s'" % form.cleaned_data["comment"]
-                models.log_action(request.user, action=action,
-                                  opportunity=opportunity)
             form.save()
+            
+            #add a new note if input by the user
+            if form.cleaned_data["new_note"].strip() != '':
+                note_text = form.cleaned_data["new_note"]
+                
+                note_log_text = note_text
+                if len(note_log_text) > 160:
+                    note_log_text = note_log_text[:160]+u"\u2026"
+                models.log_action(request.user,
+                                  action="added note: '%s'" % note_log_text,
+                                  opportunity=opportunity)
+                
+                note = Note(opportunity=opportunity, author=request.user, text=note_text)
+                note.save()
+            
             return HttpResponseRedirect(request.POST["next"])
         else:
             request.user.message_set.create(message=_('Opportunity details could not be saved.'))
     else:
         form = forms.OpportunityForm(instance=opportunity)
-    return render('opportunities/opportunity_edit.html', 
+    
+    next = '/'
+    if 'next' in request.GET: next = request.GET['next']
+    
+    return render(template, 
                     {'form': form, 
                      'opportunity':opportunity,
                      'user':request.user, 
-                     'next': request.GET['next'],
+                     'next': next,
                     }, RequestContext(request))
 
 
-def _create_packages_list(request, filters_pkg, filters_opp):
-    # XXX: rockstar: Eep! We shouldn't be storing the None as a string.  We
-    # should re-think this model relationship.
-    #sourcepackages_list = models.SourcePackage.objects.exclude(name='None')
-    
-    sourcepackages_list = models.SourcePackage.objects.distinct()
-    sourcepackages_list = filters_pkg.process_queryset(sourcepackages_list)
-    
-    #opportunities_list is filtered right away to only check opportunities belonging to selected packages
-    opportunities_list = models.Opportunity.objects.distinct().filter(sourcepackage__in=sourcepackages_list)
-    opportunities_list = filters_opp.process_queryset(opportunities_list)
-    
-    #TODO: need to filter out opportunities with valid=False again
-    #TODO: would it be more efficient to group opportunities by their sourcepackages first, then run filters_opp.process_queryset() for each of those groups?
-    
-    pkg_list_wrapper = PackageListWrapper(request, sourcepackages_list, opportunities_list)
-    
-    return pkg_list_wrapper
-
-
-def opportunities_filter(request):
-    filters = HarvestFilters()
-    filters.update_from_http(request)
-    filters_pkg = filters.find('pkg')
-    filters_opp = filters.find('opp')
-    
-    pkg_list_wrapper = _create_packages_list(request, filters_pkg, filters_opp) 
-    
-    context = {
-        'packages_list': pkg_list_wrapper,
-        'filters_pkg' : filters_pkg,
-        'filters_opp' : filters_opp
-    }
-
-    return render(
-        'opportunities/filter.html',
-        context,
-        context_instance=RequestContext(request))
-
-
-def opportunities_xhr_filter_results(request):
+
+def xhr_filter_results(request):
     filters = HarvestFilters()
     filters.update_from_http(request)
     
@@ -108,12 +137,11 @@
         context,
         context_instance=RequestContext(request))
 
-
-def opportunities_xhr_package_details(request, package_id):
+def xhr_package_details(request, package_id):
     filters = HarvestFilters()
     filters.update_from_http(request)
     
-    package = models.SourcePackage.objects.get(id=package_id)
+    package = get_object_or_404(models.SourcePackage, id=package_id)
     
     opportunities_list = filters.find('opp').process_queryset(package.opportunity_set).all()
     
@@ -128,18 +156,21 @@
         context,
         context_instance=RequestContext(request))
 
-def single_package(request, package_name):
-    package = models.SourcePackage.objects.get(name=package_name)
-    
-    package_wrapper = PackageWrapper(request, package, visible_opportunities = package.opportunity_set)
+def xhr_opportunity_li(request, opportunity_id):
+    opportunity = get_object_or_404(models.Opportunity, id=opportunity_id)
     
     context = {
-        'package': package_wrapper
+        'opportunity': opportunity
     }
     
     return render(
-        'opportunities/single_package.html',
+        'opportunities/xhr/opportunity_outer_li.html',
         context,
         context_instance=RequestContext(request))
 
+def xhr_opportunity_edit(request, opportunity_id):
+    return opportunity_edit(
+        request,
+        opportunity_id,
+        template='opportunities/xhr/opportunity_edit.html')
 
 
=== modified file 'harvest/templates/base.html'
--- harvest/templates/base.html	2010-07-21 20:21:14 +0000
+++ harvest/templates/base.html	2010-08-13 02:04:41 +0000
@@ -26,7 +26,7 @@
 <div id="container">
 
 <div id="header">
-	<span id="pagetitle">
+	<span id="sitetitle">
 		<img id="sitelogo" src="{{ MEDIA_URL }}img/logo_humanity-search-icon.png" />
 		<h1 id="sitename">{% trans "Harvest" %}</h1>
 		{% if harvest_version_name %}<span id="releasename">{{harvest_version_name}}</span>{% endif %}
@@ -59,7 +59,7 @@
 
 <div id="footer">
 	<div id="footnav"><nav>
-		<a class="title" href="{% url home %}" tabindex="2">{% trans "Harvest" %}</a> <a href="http://answers.launchpad.net/harvest" tabindex="2">{% trans "Help" %}</a> <a href="http://bugs.launchpad.net/harvest" tabindex="2">{% trans "Bugs" %}</a> <a href="http://launchpad.net/harvest" tabindex="2">{% trans "Code" %}</a>
+		<a class="title" href="{% url home %}" tabindex="2">{% trans "Harvest" %}</a> <a href="http://answers.launchpad.net/harvest" target="_blank" tabindex="2">{% trans "Help" %}</a> <a href="http://bugs.launchpad.net/harvest" target="_blank" tabindex="2">{% trans "Bugs" %}</a> <a href="http://launchpad.net/harvest" target="_blank" tabindex="2">{% trans "Code" %}</a>
 	</nav></div>
 	
 	<div id="smallprint" tabindex="3">
 
=== added file 'harvest/templates/one_column.html'
--- harvest/templates/one_column.html	1970-01-01 00:00:00 +0000
+++ harvest/templates/one_column.html	2010-08-13 02:04:41 +0000
@@ -0,0 +1,9 @@
+{% extends "base.html" %}
+{% load i18n %}
+
+{% block content %}
+<h2 class="pagetitle">{% block pagetitle %}{% endblock %}</h2>
+<div class="main">
+	{% block content_main %}{% endblock %}
+</div>
+{% endblock %}
 \ No newline at end of file
 
=== modified file 'harvest/templates/opportunities/filter.html'
--- harvest/templates/opportunities/filter.html	2010-07-30 21:45:23 +0000
+++ harvest/templates/opportunities/filter.html	2010-08-13 02:04:41 +0000
@@ -10,7 +10,7 @@
 	{{filters_opp.render}}
 </div>
 
-<div id="results-pane" data-results-url="{% url opportunities_xhr_filter_results %}">
+<div id="results-pane">
 	<div id="results-status"></div>
 	<div id="results">
 	{% include "opportunities/include/filter_results.html" %}
 
=== modified file 'harvest/templates/opportunities/include/filter_results.html'
--- harvest/templates/opportunities/include/filter_results.html	2010-07-30 08:23:24 +0000
+++ harvest/templates/opportunities/include/filter_results.html	2010-08-13 02:04:41 +0000
@@ -3,7 +3,7 @@
 {% if packages_list %}
 <ul>
 	{% for package in packages_list.get_visible_packages %}
-	<li data-results-packageid="{{ package.real.id }}" class="sourcepackage {% if package.expanded %}expanded{% else %}collapsed{% endif %}">
+	<li data-package-id="{{ package.real.id }}" class="sourcepackage {% if package.expanded %}expanded{% else %}collapsed{% endif %}">
 		<a class="sourcepackage-header" href="{{ package.get_expand_toggle_url }}">
 			<h2 class="sourcepackage-name">{{ package.real.name }}</h2>
 			<span class="status"></span>
 
=== modified file 'harvest/templates/opportunities/include/opportunity.html'
--- harvest/templates/opportunities/include/opportunity.html	2010-07-30 21:45:23 +0000
+++ harvest/templates/opportunities/include/opportunity.html	2010-08-13 02:04:41 +0000
@@ -3,16 +3,14 @@
 <div class="opportunity-header">
 	<a href="{{opportunity.url}}" class="opportunity-description" target="_blank">{{ opportunity.description }}</a>
 	{% if user.is_authenticated %}
-	<a href="{% url opportunity_edit opportunity.id %}?next={{request.get_full_path}}" class="opportunity-edit-button">edit</a>
+	<a href="{% url opportunity_edit opportunity.id %}?next={{request.get_full_path}}" target="_blank" class="opportunity-edit-button">edit</a>
 	{% endif %}
 	<span class="opportunity-summary">
 		{{ opportunity.summary|join:', ' }}
 	</span>
 </div>
 <div class="opportunity-details">
-	{% if opportunity.comment %}
-	<span class="opportunity-status">{{ opportunity.comment }}</span>
-	{% endif %}
+	{% include "opportunities/include/opportunity_details.html" %}
 </div>
 <div class="bottom"></div>
 
 
=== added file 'harvest/templates/opportunities/include/opportunity_details.html'
--- harvest/templates/opportunities/include/opportunity_details.html	1970-01-01 00:00:00 +0000
+++ harvest/templates/opportunities/include/opportunity_details.html	2010-08-13 02:04:41 +0000
@@ -0,0 +1,7 @@
+{% ifnotequal opportunity.note_set.count 0 %}
+<div class="opportunity-notes">
+	<ul>
+		{% include "opportunities/include/opportunity_notes_list.html" %}
+	</ul>
+</div>
+{% endifnotequal %}
 \ No newline at end of file
 
=== added file 'harvest/templates/opportunities/include/opportunity_details_edit.html'
--- harvest/templates/opportunities/include/opportunity_details_edit.html	1970-01-01 00:00:00 +0000
+++ harvest/templates/opportunities/include/opportunity_details_edit.html	2010-08-13 02:04:41 +0000
@@ -0,0 +1,48 @@
+{% load i18n %}
+
+<form action="{{ request.path_info }}" method="POST">
+	<div class="opportunity-notes">
+		{% with form.new_note as field %}
+		<span class="separate-top {% if field.required %}required{% endif %} {% if field.errors %}error{% endif %}">
+		{{field.errors}}
+		<input type="text" name="new_note" id="new_note" placeholder="{{field.label}}" {% if form.new_note.data %}value="{{form.new_note.data}}"{% endif %} />
+		{% endwith %}
+		</span>
+		<ul>
+			{% include "opportunities/include/opportunity_notes_list.html" %}
+		</ul>
+		<div class="bottom"></div>
+	</div>
+	
+	<ul class="opportunity-switches">
+		{% with form.experience as field %}
+		<li class="separate-top {% if field.required %}required{% endif %} {% if field.errors %}error{% endif %}">
+		{{field.errors}}
+		{{field}}
+		{{field.label_tag}}
+		</li>
+		{% endwith %}
+		
+		{% with form.applied as field %}
+		<li class="{% if field.required %}required{% endif %} {% if field.errors %}error{% endif %}">
+		{{field.errors}}
+		{{field}}
+		{{field.label_tag}}
+		</li>
+		{% endwith %}
+		
+		{% with form.reviewed as field %}
+		<li class="{% if field.required %}required{% endif %} {% if field.errors %}error{% endif %}">
+		{{field.errors}}
+		{{field}}
+		{{field.label_tag}}
+		</li>
+		{% endwith %}
+	</ul>
+	
+	<div class="actions">
+		<input type="hidden" name="next" value="{{ next }}" />
+		<input type="submit" value="{% trans 'Apply changes' %}" />
+		<a href="{{next}}" rel="deactivate"><button>Cancel</button></a>
+	</div>
+</form>
 
=== added file 'harvest/templates/opportunities/include/opportunity_li.html'
--- harvest/templates/opportunities/include/opportunity_li.html	1970-01-01 00:00:00 +0000
+++ harvest/templates/opportunities/include/opportunity_li.html	2010-08-13 02:04:41 +0000
@@ -0,0 +1,3 @@
+<li class="opportunity" data-opportunity-experience="{{opportunity.experience}}" {% if opportunity.reviewed %}data-opportunity-irrelevant{% endif %} {% if opportunity.applied %}data-opportunity-applied{% endif %}>
+	{% include "opportunities/include/opportunity.html" %}
+</li>
 
=== added file 'harvest/templates/opportunities/include/opportunity_notes_list.html'
--- harvest/templates/opportunities/include/opportunity_notes_list.html	1970-01-01 00:00:00 +0000
+++ harvest/templates/opportunities/include/opportunity_notes_list.html	2010-08-13 02:04:41 +0000
@@ -0,0 +1,9 @@
+{% load i18n %}
+{% load humanize_timediff %}
+
+{% for note in opportunity.note_set.all|dictsortreversed:"date" %}
+<li>
+	{{ note.text }}
+	<span class="signature">{% blocktrans with note.date|humanize_timediff as timesince and note.author as author %}{{author}}, {{timesince}} ago{% endblocktrans %}</span>
+</li>
+{% endfor %}
 
=== added file 'harvest/templates/opportunities/include/opportunity_outer_li.html'
--- harvest/templates/opportunities/include/opportunity_outer_li.html	1970-01-01 00:00:00 +0000
+++ harvest/templates/opportunities/include/opportunity_outer_li.html	2010-08-13 02:04:41 +0000
@@ -0,0 +1,3 @@
+<li class="opportunity" data-opportunity-id="{{opportunity.id}}" data-opportunity-experience="{{opportunity.experience}}" {% if opportunity.reviewed %}data-opportunity-irrelevant{% endif %} {% if opportunity.applied %}data-opportunity-applied{% endif %}>
+	{% include "opportunities/include/opportunity.html" %}
+</li>
 
=== modified file 'harvest/templates/opportunities/include/package_details.html'
--- harvest/templates/opportunities/include/package_details.html	2010-07-31 20:22:58 +0000
+++ harvest/templates/opportunities/include/package_details.html	2010-08-13 02:04:41 +0000
@@ -11,9 +11,7 @@
 <ul>
 	{# FIXME: use |dictsort:'experience'|dictsort:'description' here (see comment in wrappers.py) #}
 	{% for opportunity in opplist.list %}
-	<li class="opportunity" data-opportunity-experience="{{opportunity.experience}}" {% if opportunity.reviewed %}data-opportunity-irrelevant{% endif %} {% if opportunity.applied %}data-opportunity-applied{% endif %}>
-		{% include "opportunities/include/opportunity.html" %}
-	</li>
+	{% include "opportunities/include/opportunity_outer_li.html" %}
 	{% endfor %}
 </ul>
 </div>
 
=== modified file 'harvest/templates/opportunities/opportunity_edit.html'
--- harvest/templates/opportunities/opportunity_edit.html	2010-07-30 08:23:24 +0000
+++ harvest/templates/opportunities/opportunity_edit.html	2010-08-13 02:04:41 +0000
@@ -1,17 +1,14 @@
-{% extends "base.html" %}
+{% extends "one_column.html" %}
 {% load i18n %}
 
-{% block content %}
-    <div class="opportunity">
-        {% include "opportunities/include/opportunity.html" %}
-    </div>
-    <div id="form"><form action="{{ request.path_info }}" method="POST">
-        {% if form.errors %}
-             <p style="color: red;">{% trans  "Please correct the error" %} {{ form.errors|pluralize }} below.</p>
-        {% endif %}
-        <table>{{ form.as_table }}</table>
-	<td><input type="hidden" name="next" value="{{ next }}" /> </td>
-	<td><input type="submit" value='{% trans "Update Information Now!" %}' /></td>
-	<td><a href="{{next}}" rel="deactivate"><button>Cancel</button></a></td>
-    </form></div>
+{% block title %}{{ block.super }}: {{ opportunity.description }}{% endblock %}
+
+{% block pagetitle %}
+Opportunity <a href="{{opportunity.url}}" class="opportunity-description" target="_blank">{{ opportunity.description }}</a> in {% with opportunity.sourcepackage.name as pkgname %}<a href="{% url single_package pkgname %}">{{ pkgname }}</a>{% endwith %}
+{% endblock %}
+
+{% block content_main %}
+<div class="opportunity-details edit">
+	{% include "opportunities/include/opportunity_details_edit.html" %}
+</div>
 {% endblock %}
 
=== modified file 'harvest/templates/opportunities/single_package.html'
--- harvest/templates/opportunities/single_package.html	2010-07-30 21:45:23 +0000
+++ harvest/templates/opportunities/single_package.html	2010-08-13 02:04:41 +0000
@@ -1,20 +1,13 @@
-{% extends "base.html" %}
+{% extends "one_column.html" %}
 {% load i18n %}
 
 {% block title %}{{ block.super }}: {{ package.real.name }}{% endblock %}
 
-{% block content %}
+{% block pagetitle %}{{ package.real.name }}{% endblock %}
 
-<div class="sourcepackage">
-	<div class="sourcepackage-header">
-		<h2 class="sourcepackage-name">{{ package.real.name }}</h2>
-		{% comment %}<span class="sourcepackage-summary"></span>{% endcomment %}
-		<div class="bottom"></div>
-	</div>
-	<div class="sourcepackage-details">
-		{% include "opportunities/include/package_details.html" %}
-	</div>
+{% block content_main %}
+<div class="sourcepackage-details">
+	{% include "opportunities/include/package_details.html" %}
 </div>
-
 {% endblock %}
 
 
=== added file 'harvest/templates/opportunities/xhr/opportunity_edit.html'
--- harvest/templates/opportunities/xhr/opportunity_edit.html	1970-01-01 00:00:00 +0000
+++ harvest/templates/opportunities/xhr/opportunity_edit.html	2010-08-13 02:04:41 +0000
@@ -0,0 +1,3 @@
+{% load i18n %}
+{% include "opportunities/include/opportunity_details_edit.html" %}
+
 
=== added file 'harvest/templates/opportunities/xhr/opportunity_li.html'
--- harvest/templates/opportunities/xhr/opportunity_li.html	1970-01-01 00:00:00 +0000
+++ harvest/templates/opportunities/xhr/opportunity_li.html	2010-08-13 02:04:41 +0000
@@ -0,0 +1,3 @@
+{% load i18n %}
+{% include "opportunities/include/opportunity_li.html" %}
+
 
=== added file 'harvest/templates/opportunities/xhr/opportunity_outer_li.html'
--- harvest/templates/opportunities/xhr/opportunity_outer_li.html	1970-01-01 00:00:00 +0000
+++ harvest/templates/opportunities/xhr/opportunity_outer_li.html	2010-08-13 02:04:41 +0000
@@ -0,0 +1,3 @@
+{% load i18n %}
+{% include "opportunities/include/opportunity_outer_li.html" %}
+