Hey, On 11/04/2015 10:04 PM, Raphael Hertzog wrote: > Hello Orestis, > > sorry that I did not find the time to get back to you earlier. >
no worries ;) > 1/ that the package page contains a link to the package news page (the > link you added in the header of the panel) > 2/ that the package page contains a link to the "?page=2" version of the > package news page when you have more news than the configured limit (and > possibly that you don't have any such link when you're below) ack for those 2 > 3/ the package news page does return the expected content for "?page=2" > just to ensure that pagination works and that we display something that > looks OK > Added a test, I am not sure though if that's what you were looking for.. Fixed the other suggestions as well.. Sending a patch for a flake8 issue. Cheers, Orestis
From e0163800224b5d3e490b6358b25cb37868031fd2 Mon Sep 17 00:00:00 2001
From: Orestis Ioannou <ores...@oioannou.com>
Date: Wed, 2 Sep 2015 10:38:10 +0200
Subject: [PATCH 1/2] Add support for browsing package news
Packages now have their own specific news page accessible by
the respective panel in the package page.
---
distro_tracker/core/panels.py | 6 ++-
.../core/templates/core/_news_pagination.html | 34 +++++++++++++++
.../core/templates/core/package_news.html | 8 ++++
.../core/templates/core/panels/news.html | 31 +++-----------
distro_tracker/core/tests/tests_panels.py | 43 +++++++++++++++++++
distro_tracker/core/tests/tests_views.py | 50 ++++++++++++++++++++++
distro_tracker/core/views.py | 28 ++++++++++++
distro_tracker/project/urls.py | 4 ++
8 files changed, 178 insertions(+), 26 deletions(-)
create mode 100644 distro_tracker/core/templates/core/_news_pagination.html
create mode 100644 distro_tracker/core/templates/core/package_news.html
diff --git a/distro_tracker/core/panels.py b/distro_tracker/core/panels.py
index 8c2cd82..08d3dc2 100644
--- a/distro_tracker/core/panels.py
+++ b/distro_tracker/core/panels.py
@@ -805,9 +805,11 @@ class NewsPanel(BasePanel):
def context(self):
news = News.objects.prefetch_related('signed_by')
news = news.filter(package=self.package).order_by('-datetime_created')
- news = news[:self.NEWS_LIMIT]
+ news = list(news[:self.NEWS_LIMIT])
+ more_pages = len(news) == self.NEWS_LIMIT
return {
- 'news': news
+ 'news': news,
+ 'has_more': more_pages
}
@property
diff --git a/distro_tracker/core/templates/core/_news_pagination.html b/distro_tracker/core/templates/core/_news_pagination.html
new file mode 100644
index 0000000..82d4f9a
--- /dev/null
+++ b/distro_tracker/core/templates/core/_news_pagination.html
@@ -0,0 +1,34 @@
+<ul class="list-group list-group-flush">
+ {% for news_item in news %}
+ <li class="list-group-item">
+ [<span class="news-date">{{ news_item.datetime_created|date:"Y-m-d" }}</span>]
+ <a href="{% url 'dtracker-news-page' news_item.pk %}">
+ <span class="news-title">{{ news_item.title }}</span>
+ </a>
+ {% if news_item.created_by %}(<span class="news-creator">{{ news_item.created_by }}</span>){% endif %}
+ {% with signers=news_item.signed_by.all %}
+ {% if signers and signers.0.name != news_item.created_by %}
+ {% spaceless %}
+ <span>(signed by: </span>
+ {% for signer in signers %}
+ <span class="news-signer">{{ signer.name }}</span>
+ {% if not forloop.last %}<span>, </span>{% endif %}
+ {% endfor %}
+ <span>)</span>
+ {% endspaceless %}
+ {% endif %}
+ {% endwith %}
+ </li>
+ {% endfor %}
+</ul>
+{% if is_paginated %}
+<div class="text-center">
+<div class="pagination">
+<ul>
+ {% for page in page_obj.paginator.page_range %}
+ <li {% if page_obj.number == page %}class="active"{% endif %}><a href="?page={{ page }}">{{ page }}</a></li>
+ {% endfor %}
+</ul>
+</div>
+</div>
+{% endif %}
diff --git a/distro_tracker/core/templates/core/package_news.html b/distro_tracker/core/templates/core/package_news.html
new file mode 100644
index 0000000..ed40eaf
--- /dev/null
+++ b/distro_tracker/core/templates/core/package_news.html
@@ -0,0 +1,8 @@
+{% extends 'core/base.html' %}
+
+{% block content %}
+<h1 class="text-center">News for package <a href="{% url 'dtracker-package-page' package %}">{{ package }}</a></h1>
+<div class="row-fluid">
+{% include "core/_news_pagination.html" %}
+</div>
+{% endblock %}
diff --git a/distro_tracker/core/templates/core/panels/news.html b/distro_tracker/core/templates/core/panels/news.html
index 1cc38fd..b0532ed 100644
--- a/distro_tracker/core/templates/core/panels/news.html
+++ b/distro_tracker/core/templates/core/panels/news.html
@@ -3,7 +3,7 @@
{% block panel-header %}
<div class="row-fluid">
<div class="pull-left">
- <span>{{ panel.title }}</span>
+ <span>{{ panel.title }} <a href="{% url 'dtracker-package-news' package.name %}"><i class='icon-share'></i></a></span>
</div>
<div class="pull-right">
<a title="rss feed" href="{% url 'dtracker-package-rss-news-feed' package.name %}">
@@ -14,27 +14,10 @@
{% endblock %}
{% block panel-body %}
-<ul class="list-group list-group-flush">
- {% for news_item in panel.context.news %}
- <li class="list-group-item">
- [<span class="news-date">{{ news_item.datetime_created|date:"Y-m-d" }}</span>]
- <a href="{% url 'dtracker-news-page' news_item.pk %}">
- <span class="news-title">{{ news_item.title }}</span>
- </a>
- {% if news_item.created_by %}(<span class="news-creator">{{ news_item.created_by }}</span>){% endif %}
- {% with signers=news_item.signed_by.all %}
- {% if signers and signers.0.name != news_item.created_by %}
- {% spaceless %}
- <span>(signed by: </span>
- {% for signer in signers %}
- <span class="news-signer">{{ signer.name }}</span>
- {% if not forloop.last %}<span>, </span>{% endif %}
- {% endfor %}
- <span>)</span>
- {% endspaceless %}
- {% endif %}
- {% endwith %}
- </li>
- {% endfor %}
-</ul>
+{% with news=panel.context.news %}
+ {% include "core/_news_pagination.html" %}
+{% endwith %}
+{% if panel.context.has_more %}
+<div class="pagination">Page <a href="{% url 'dtracker-package-news' package.name %}?page=2">2</a></div>
+{% endif %}
{% endblock %}
diff --git a/distro_tracker/core/tests/tests_panels.py b/distro_tracker/core/tests/tests_panels.py
index bf5c91b..f64a471 100644
--- a/distro_tracker/core/tests/tests_panels.py
+++ b/distro_tracker/core/tests/tests_panels.py
@@ -219,3 +219,46 @@ class DeadPackageWarningPanelTests(TestCase):
SourcePackageRepositoryEntry.objects.create(
source_package=self.srcpkg, repository=self.repo1)
self.assertFalse(self.panel.context['disappeared'])
+
+
+class NewsPanelTests(TestCase):
+ def setUp(self):
+ self.package = SourcePackageName.objects.create(name='dummy-package')
+ self.src_pkg = SourcePackage.objects.create(
+ source_package_name=self.package, version='1.0.0')
+ self.src_pkg.save()
+ # add some news
+ for i in range(29):
+ self.package.news_set.create(title="News {}".format(i),
+ created_by="Author {}".format(i))
+
+ def get_package_page_response(self):
+ url = reverse('dtracker-package-page', kwargs={
+ 'package_name': self.package.name,
+ })
+ return self.client.get(url)
+
+ def find_link_in_content(self, content, link):
+ """Helper method to parse response and verify link exists"""
+ html = soup(content)
+ for a_tag in html.findAll('a', {'href': True}):
+ if a_tag['href'] == link:
+ return True
+ return False
+
+ def test_news_links(self):
+ """Tests the links in the news panel"""
+ response = self.get_package_page_response()
+ news_link = reverse('dtracker-package-news', kwargs={
+ 'package_name': self.package.name})
+ # verify news-link is present
+ self.assertTrue(self.find_link_in_content(response.content, news_link))
+ # verify ?page=2 not present
+ self.assertFalse(self.find_link_in_content(response.content,
+ news_link + "?page=2"))
+ self.package.news_set.create(title="News {}".format(30),
+ created_by="Author {}".format(30))
+ # refresh package page to include new news item
+ response = self.get_package_page_response()
+ self.assertTrue(self.find_link_in_content(response.content,
+ news_link + "?page=2"))
diff --git a/distro_tracker/core/tests/tests_views.py b/distro_tracker/core/tests/tests_views.py
index e45c116..cde56be 100644
--- a/distro_tracker/core/tests/tests_views.py
+++ b/distro_tracker/core/tests/tests_views.py
@@ -25,6 +25,7 @@ import json
from django.core.urlresolvers import reverse
from django.conf import settings
+from bs4 import BeautifulSoup as soup
import os
@@ -477,3 +478,52 @@ class ActionItemJsonViewTest(TestCase):
}))
self.assertEqual(response.status_code, 404)
+
+
+class NewsViewTest(TestCase):
+ """
+ Tests for the :class:`distro_tracker.core.views.PackageNews`.
+ """
+ def setUp(self):
+ self.package = SourcePackageName.objects.create(name='dummy-package')
+ self.src_pkg = SourcePackage.objects.create(
+ source_package_name=self.package, version='1.0.0')
+ self.src_pkg.save()
+ # add some news
+ for i in range(61):
+ self.package.news_set.create(title="News {}".format(i),
+ created_by="Author {}".format(i))
+
+ def get_package_news(self, url, page=None):
+ if not page:
+ return self.client.get(url)
+ else:
+ return self.client.get('%s?page=%s' % (url, page))
+
+ def find_link_in_content(self, content, link):
+ """Helper method to parse response and verify link exists"""
+ html = soup(content)
+ for a_tag in html.findAll('a', {'href': True}):
+ if a_tag['href'] == link:
+ return True
+ return False
+
+ def test_news_page(self):
+ """ Tests that the second news page is what we expect"""
+ url = reverse('dtracker-package-news', kwargs={
+ 'package_name': self.package.name})
+ response = self.get_package_news(url)
+ # verify we can return to package page
+ package_url = reverse('dtracker-package-page', kwargs={
+ 'package_name': self.package.name,
+ })
+ self.assertTrue(self.find_link_in_content(response.content,
+ package_url))
+ # verify we only supply valid link pages
+ self.assertTrue(self.find_link_in_content(response.content,
+ '?page=2'))
+ self.assertFalse(self.find_link_in_content(response.content,
+ '?page=4'))
+ response = self.get_package_news(url, 2)
+ self.assertTrue(self.find_link_in_content(response.content,
+ '?page=1'))
diff --git a/distro_tracker/core/views.py b/distro_tracker/core/views.py
index 16b958b..593bb4b 100644
--- a/distro_tracker/core/views.py
+++ b/distro_tracker/core/views.py
@@ -176,6 +176,34 @@ def news_page(request, news_id):
})
+class PackageNews(ListView):
+ """
+ A view which lists all the news of a package.
+ """
+ _DEFAULT_NEWS_LIMIT = 30
+ NEWS_LIMIT = getattr(
+ settings,
+ 'DISTRO_TRACKER_NEWS_PANEL_LIMIT',
+ _DEFAULT_NEWS_LIMIT)
+
+ paginate_by = NEWS_LIMIT
+ template_name = 'core/package_news.html'
+ context_object_name = 'news'
+
+ def get(self, request, package_name):
+ self.package = get_object_or_404(PackageName, name=package_name)
+ return super(PackageNews, self).get(request, package_name)
+
+ def get_queryset(self):
+ news = self.package.news_set.prefetch_related('signed_by')
+ return news.order_by('-datetime_created')
+
+ def get_context_data(self, *args, **kwargs):
+ context = super(PackageNews, self).get_context_data(*args, **kwargs)
+ context['package'] = self.package
+ return context
+
+
class ActionItemJsonView(View):
"""
View renders a :class:`distro_tracker.core.models.ActionItem` in a JSON
diff --git a/distro_tracker/project/urls.py b/distro_tracker/project/urls.py
index cbe1fe8..0b21808 100644
--- a/distro_tracker/project/urls.py
+++ b/distro_tracker/project/urls.py
@@ -39,6 +39,7 @@ from distro_tracker.core.views import SetMuteTeamView
from distro_tracker.core.views import SetMembershipKeywords
from distro_tracker.core.views import EditMembershipView
from distro_tracker.core.views import IndexView
+from distro_tracker.core.views import PackageNews
from distro_tracker.core.news_feed import PackageNewsFeed
from distro_tracker.accounts.views import ConfirmAddAccountEmail
from distro_tracker.accounts.views import LoginView
@@ -213,6 +214,9 @@ urlpatterns = patterns(
url(r'^teams/(?P<slug>.+?)/$', TeamDetailsView.as_view(),
name='dtracker-team-page'),
+ # Package news page
+ url(r'^pkg/(?P<package_name>.+)/news/', PackageNews.as_view(),
+ name='dtracker-package-news'),
# Dedicated package page
url(r'^pkg/(?P<package_name>[^/]+)/?$',
--
2.6.2
From 93641f8e94451cc6f2d5e4e7046eba84e650514d Mon Sep 17 00:00:00 2001
From: Orestis Ioannou <ores...@oioannou.com>
Date: Tue, 10 Nov 2015 18:52:29 +0100
Subject: [PATCH 2/2] Fix flake8 issue
---
distro_tracker/vendor/debian/tracker_tasks.py | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/distro_tracker/vendor/debian/tracker_tasks.py b/distro_tracker/vendor/debian/tracker_tasks.py
index 6a7e5b2..617bdf1 100644
--- a/distro_tracker/vendor/debian/tracker_tasks.py
+++ b/distro_tracker/vendor/debian/tracker_tasks.py
@@ -424,8 +424,8 @@ class UpdatePackageBugStats(BaseTask):
src, package_name, bug_counts = line.split(':', 2)
else:
package_name, bug_counts = line.split(':', 1)
- # Merged counts are in parentheses so remove those before splitting
- # the numbers
+ # Merged counts are in parentheses so remove those before
+ # splitting the numbers
bug_counts = re.sub(r'[()]', ' ', bug_counts).split()
bug_counts = [int(count) for count in bug_counts]
except ValueError:
--
2.6.2
signature.asc
Description: OpenPGP digital signature
