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.

Sign up for GitHub

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

Feature/sort talks page #717

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
56b932d
Add language and track to create_talk
Jul 22, 2024
9a79f10
Add parameter to sort talks
Jul 22, 2024
ccd47f0
Add test for sorting behavior
Jul 22, 2024
479ceba
Add unsorted case to tests
Jul 24, 2024
62aded5
Pass sorted flag back to template for links
Aug 19, 2024
390d633
Add clickable headers to the table for sorting
Aug 19, 2024
4b8f37c
Pass sort flag through to pagination
Aug 19, 2024
4edc96d
Update wafer/talks/views.py
drnlm Aug 19, 2024
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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 •
edited
Loading

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
19 changes: 17 additions & 2 deletions wafer/talks/views.py
View file
Open in desktop
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,31 @@ 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)
if self.request.GET.get('sort'):
context['sort'] = self.request.GET.get('sort')
else:
context['sort'] = 'default'
drnlm marked this conversation as resolved.
Show resolved Hide resolved
return context


Expand Down
Loading