harvest

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

  1. bug518887
  2. Merge into trunk
Proposed by Daniel Holbach
Status: Merged
Approved by: James Westby
Approved revision: 180
Merged at revision: 187
Proposed branch: lp:~dholbach/harvest/bug518887
Merge into: lp:harvest
Diff against target: 60 lines (+1/-40)
1 file modified
harvest/opportunities/templates/opportunities/opportunities_by_type.html (+1/-40)
To merge this branch: bzr merge lp:~dholbach/harvest/bug518887
Reviewer Review Type Date Requested Status
James Westby Approve
Paul Hummer (community) Needs Information
Review via email: mp+21548@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Daniel Holbach (dholbach) wrote :

Hello? :-)

Revision history for this message
Dylan McCall (dylanmccall) wrote :

It is definitely a victory: it brings the number of SQL queries down to 20, and the elapsed time (as reported by debug-toolbar) to 200ms. Previously, it was immeasurably huge.

This week I'm working to replace the list templates altogether in my branch, though, so I think it's pretty well moot :)

Revision history for this message
Paul Hummer (rockstar) wrote :

Daniel-

  It's best to give some context when you propose a branch for merging. I'm not entirely sure what this branch really does. Sure, it cuts down the number of SQL queries, but it also removes some functionality, and I'm not sure why it is. Specifically, why are we no longer paginating things?

Cheers,
Paul

review: Needs Information
Revision history for this message
Daniel Holbach (dholbach) wrote :

Paul: We just show 10-15 "opportunity lists" right now, so they don't get paginated.

Revision history for this message
James Westby (james-w) wrote :

Looks good.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'harvest/opportunities/templates/opportunities/opportunities_by_type.html'
2--- harvest/opportunities/templates/opportunities/opportunities_by_type.html 2010-02-22 15:43:51 +0000
3+++ harvest/opportunities/templates/opportunities/opportunities_by_type.html 2010-03-17 11:54:18 +0000
4@@ -13,55 +13,16 @@
5 {% for type in types.object_list %}
6 <li>
7 <div class="package_name threshold{{ type.opportunity_class }}">
8- <span class="name">{{ type.name }}</span>
9+ <span class="name"><a href="{{ type.get_absolute_url }}">{{ type.name }}</a></span>
10 <span class="count">
11 ({{ type.opportunity_set.count }} issues)
12 </span>
13 </div>
14- <div class="package_details">
15- <ul>
16- {% for opportunity in type.opportunity_set.all %}
17- {% include "opportunities/opportunity_detail.inc.html" %}
18- {% endfor %}
19- </ul>
20- </div>
21- </li>
22 {% endfor %}
23 </ul>
24-
25- <div class="pagination">
26- <span class="step-links">
27- {% if types.has_previous %}
28- <a href="?page={{ types.previous_page_number }}">previous</a>
29- {% endif %}
30-
31- <span class="current">
32- Page {{ types.number }} of {{ types.paginator.num_pages }}.
33- </span>
34-
35- {% if types.has_next %}
36- <a href="?page={{ types.next_page_number }}">next</a>
37- {% endif %}
38- </span>
39- </div>
40-
41 {% else %}
42 <p>{% trans "There are currently no opportunities in Harvest. :(" %}</p>
43 {% endif %}
44
45 </div>
46-
47-<script type="text/javascript">
48-YUI().use('node', function(Y) {
49-
50- Y.all('.package_name').on('click', function(e) {
51- // XXX: rockstar: Currently, this is kinda jumpy. It'd be nice if it was
52- // all sexy animated.
53- Y.all('.package_details').setStyle('display', 'none');
54- e.currentTarget.next().setStyle('display', 'block');
55- });
56-
57-});
58-
59-</script>
60 {% endblock %}

Subscribers

People subscribed via source and target branches

to all changes: