Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature/sort talks page #717

Merged
merged 8 commits into from
Aug 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 24 additions & 10 deletions wafer/talks/templates/wafer.talks/talks.html
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,28 @@ <h1>{% trans "Accepted Talks" %}</h1>
<thead class='thead-dark'>
<tr>
<th>
{% if type %}
{{ type }}
{% else %}
{% trans "Talk" %}
{% endif %}
<a href="{% url 'wafer_users_talks' %}?sort={% if sort == 'title' %}default{% else %}title{% endif %}">
{% if type %}
{{ type }}
{% else %}
{% trans "Talk" %}
{% endif %}
</a>
</th>
{% if tracks %}<th>{% trans "Track" %}</th>{% endif %}
{% if languages %}<th>{% trans "Language" %}</th>{% endif %}
{% if tracks %}
<th>
<a href="{% url 'wafer_users_talks' %}?sort={% if sort == 'track' %}default{% else %}track{% endif %}">
{% trans "Track" %}
</a>
</th>
{% endif %}
{% if languages %}
<th>
<a href="{% url 'wafer_users_talks' %}?sort={% if sort == 'lang' %}default{% else %}lang{% endif %}">
{% trans "Language" %}
</a>
</th>
{% endif %}
<th>{% trans "Speakers" %}</th>
</tr>
</thead>
Expand Down Expand Up @@ -90,15 +104,15 @@ <h1>{% trans "Accepted Talks" %}</h1>
<section class="wafer wafer-pagination">
<ul class="pagination">
{% if page_obj.has_previous %}
<li class="page-item"><a class="page-link" href="{% url 'wafer_users_talks_page' page=page_obj.previous_page_number %}">&laquo;</a></li>
<li class="page-item"><a class="page-link" href="{% url 'wafer_users_talks_page' page=page_obj.previous_page_number %}?sort={{ sort }}">&laquo;</a></li>
{% else %}
<li class="page-item" class="disabled"><a class="page-link" href="#">&laquo;</a></li>
{% endif %}
{% for page in paginator.page_range %}
<li class="page-item"><a class="page-link" href="{% url 'wafer_users_talks_page' page=page %}">{{ page }}</a></li>
<li class="page-item"><a class="page-link" href="{% url 'wafer_users_talks_page' page=page %}?sort={{ sort }}">{{ page }}</a></li>
{% endfor %}
{% if page_obj.has_next %}
<li class="page-item"><a class="page-link" href="{% url 'wafer_users_talks_page' page=page_obj.next_page_number %}">&raquo;</a></li>
<li class="page-item"><a class="page-link" href="{% url 'wafer_users_talks_page' page=page_obj.next_page_number %}?sort={{ sort }}">&raquo;</a></li>
{% else %}
<li class="page-item" class="disabled"><a class="page-link" href="#">&raquo;</a></li>
{% endif %}
Expand Down
9 changes: 8 additions & 1 deletion wafer/talks/tests/fixtures.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@ def create_talk_type(name, **kwargs):
return TalkType.objects.create(name=name, **kwargs)


def create_talk(title, status, username=None, user=None, talk_type=None):
def create_talk(title, status, username=None, user=None, talk_type=None,
talk_track=None, language=None):
if sum((user is None, username is None)) != 1:
raise ValueError('One of user OR username must be specified')
if username:
Expand All @@ -21,4 +22,10 @@ def create_talk(title, status, username=None, user=None, talk_type=None):
if talk_type:
talk.talk_type = talk_type
talk.save()
if talk_track:
talk.track = talk_track
talk.save()
if language:
talk.language = language
talk.save()
return talk
88 changes: 84 additions & 4 deletions wafer/talks/tests/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -89,8 +89,8 @@ def test_no_options(self):
self.assertNotContains(response, 'Language', html=True)
self.assertNotContains(response, '<td colspan="2">\nNo talks accepted yet.\n</td>', html=True)

def test_langauge(self):
"""Test that we have the right layout if langauges, but no tracks"""
def test_language(self):
"""Test that we have the right layout if languages, but no tracks"""
Talk.LANGUAGES = (('en', 'English'),)

response = self.client.get('/talks/')
Expand All @@ -114,7 +114,7 @@ def test_langauge(self):

def test_track(self):
"""Test that we have the right layout if tracks, but no languages"""
track1 = Track.objects.create(name="Test Trakc 1")
track1 = Track.objects.create(name="Test Track 1")
response = self.client.get('/talks/')
self.assertTrue(response.context['tracks'])
self.assertEqual(len(response.context['languages']), 0)
Expand All @@ -137,7 +137,7 @@ def test_track(self):

def test_track_and_language(self):
"""Test that we have the right layout if we have tracks and languages"""
track1 = Track.objects.create(name="Test Trakc 1")
track1 = Track.objects.create(name="Test Track 1")
Talk.LANGUAGES = (('en', 'English'),)

response = self.client.get('/talks/')
Expand All @@ -160,6 +160,86 @@ def test_track_and_language(self):
self.assertNotContains(response, '<td colspan="4">\nNo talks accepted yet.\n</td>', html=True)


class TalksListSortingTests(TestCase):
"""Test the table sorting options.

This monkey-patches TALKS.languages for the same reason as the layout tests.
"""

def setUp(self):
"""Create talks and tracks"""
self.client = Client()
Talk.LANGUAGES = (('en', 'English'), ('de', 'German'))

author = create_user('author_a')

# We choose the names so that they sort differently from creation order
track2 = Track.objects.create(name="Test Track 2", order=2)
track1 = Track.objects.create(name="Test Track 1", order=1)

talk_d = create_talk('Talk D', ACCEPTED, user=author, talk_track=track1,
language='en')
talk_c = create_talk('Talk C', ACCEPTED, user=author, talk_track=track2,
language='de')
talk_b = create_talk('Talk B', ACCEPTED, user=author, talk_track=track1,
language='de')
talk_a = create_talk('Talk A', ACCEPTED, user=author, talk_track=track2,
language='en')

def tearDown(self):
"""Ensure Talk monkey-patching doesn't leak"""
Talk.LANGUAGES = ()

def test_no_sorting(self):
response = self.client.get('/talks/')
# We check via find to avoid hardcoding too much HTML here
pos_talk_a = response.content.find(b'Talk A')
pos_talk_b = response.content.find(b'Talk B')
pos_talk_c = response.content.find(b'Talk C')
pos_talk_d = response.content.find(b'Talk D')
# Order should be D, C, B, A (id order)
self.assertGreater(pos_talk_a, pos_talk_b)
self.assertGreater(pos_talk_b, pos_talk_c)
self.assertGreater(pos_talk_c, pos_talk_d)
Comment on lines +201 to +203
Copy link
Member

@hodgestar hodgestar Aug 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible to make this assert closer to self.assertTrue(D < C < B < A)?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That construction loses information about which specific item was out of order if the test fails.


def test_sort_title(self):
response = self.client.get('/talks/?sort=title')
pos_talk_a = response.content.find(b'Talk A')
pos_talk_b = response.content.find(b'Talk B')
pos_talk_c = response.content.find(b'Talk C')
pos_talk_d = response.content.find(b'Talk D')
# Should be A, B, C, D
self.assertGreater(pos_talk_b, pos_talk_a)
self.assertGreater(pos_talk_c, pos_talk_b)
self.assertGreater(pos_talk_d, pos_talk_c)

def test_sort_track(self):
response = self.client.get('/talks/?sort=track')
pos_talk_a = response.content.find(b'Talk A')
pos_talk_b = response.content.find(b'Talk B')
pos_talk_c = response.content.find(b'Talk C')
pos_talk_d = response.content.find(b'Talk D')

# track 1 (B, D) should sort before track 2 (A, C)
self.assertGreater(pos_talk_a, pos_talk_b)
self.assertGreater(pos_talk_c, pos_talk_b)
self.assertGreater(pos_talk_a, pos_talk_d)
self.assertGreater(pos_talk_c, pos_talk_d)

def test_sort_lang(self):
response = self.client.get('/talks/?sort=lang')
pos_talk_a = response.content.find(b'Talk A')
pos_talk_b = response.content.find(b'Talk B')
pos_talk_c = response.content.find(b'Talk C')
pos_talk_d = response.content.find(b'Talk D')

# de (B, C) should sort before en (A, D)
self.assertGreater(pos_talk_a, pos_talk_b)
self.assertGreater(pos_talk_a, pos_talk_c)
self.assertGreater(pos_talk_d, pos_talk_b)
self.assertGreater(pos_talk_d, pos_talk_c)


class TalkViewTests(TestCase):
def setUp(self):
public_type = create_talk_type(name="BoF", show_pending_submissions=True)
Expand Down
16 changes: 14 additions & 2 deletions wafer/talks/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@ class UsersTalks(PaginatedBuildableListView):
build_prefix = 'talks'
paginate_by = 100

@order_results_by('talk_type', 'talk_id')
def get_queryset(self):
# self.request will be None when we come here via the static site
# renderer
Expand All @@ -57,15 +56,28 @@ def get_queryset(self):
| Q(status__in=(SUBMITTED, UNDER_CONSIDERATION, PROVISIONAL),
talk_type__show_pending_submissions=True)
)
if self.request:
if self.request.GET.get('sort') == 'track' and Track.objects.count() > 0:
talks = talks.order_by('talk_type', 'track')
elif self.request.GET.get('sort') == 'lang' and Talk.LANGUAGES:
talks = talks.order_by('talk_type', 'language')
elif self.request.GET.get('sort') == 'title':
talks = talks.order_by('talk_type', 'title')
else:
talks = talks.order_by('talk_type', 'talk_id')
else:
talks = talks.order_by('talk_type', 'talk_id')
return talks.prefetch_related(
"talk_type", "corresponding_author", "authors", "authors__userprofile"
"talk_type", "corresponding_author", "authors", "authors__userprofile",
"track"
)

def get_context_data(self, **kwargs):
context = super().get_context_data(**kwargs)
context["languages"] = Talk.LANGUAGES
context["tracks"] = Track.objects.count() > 0
context["see_all"] = Talk.can_view_all(self.request.user)
context['sort'] = self.request.GET.get('sort', 'default')
return context


Expand Down
Loading