Skip to content

Commit

Permalink
🪲 Fix pagination links (#6075)
Browse files Browse the repository at this point in the history
The pagination links in various pages were using `url_for('programs_page')`, which builds a URL to a route on the global `app` object.

Since #6052 we don't have a global app object anymore. Instead, we use a global Blueprint called `app` instead.

In this PR, make all `url_for` calls in the same `app.py` file Blueprint-relative by prepending a period: `url_for('.programs_page')`, and make the `url_for` in the teachers page that points to a user's programs page (probably legacy and unused?) absolute by prepending the full Blueprint name: `url_for('app.programs_page')`.

Add tests to make sure that the page renders successfully.

**How to test**

Automatic tests have been added, so if they pass we should be good.
  • Loading branch information
rix0rrr authored Dec 27, 2024
1 parent 4a234e6 commit bdbbcc2
Show file tree
Hide file tree
Showing 7 changed files with 111 additions and 15 deletions.
12 changes: 6 additions & 6 deletions app.py
Original file line number Diff line number Diff line change
Expand Up @@ -1126,9 +1126,9 @@ def programs_page(user):
sorted_adventure_programs = hedy_content.Adventures(g.lang) \
.get_sorted_adventure_programs(all_programs, adventure_names)

next_page_url = url_for('programs_page', **dict(request.args, page=result.next_page_token)
next_page_url = url_for('.programs_page', **dict(request.args, page=result.next_page_token)
) if result.next_page_token else None
prev_page_url = url_for('programs_page', **dict(request.args, page=result.prev_page_token)
prev_page_url = url_for('.programs_page', **dict(request.args, page=result.prev_page_token)
) if result.prev_page_token else None

return render_template(
Expand Down Expand Up @@ -2473,9 +2473,9 @@ def explore():
language_filter=language,
adventure_filter=adventure,
pagination_token=page)
next_page_url = url_for('explore', **dict(request.args, page=result.next_page_token)
next_page_url = url_for('.explore', **dict(request.args, page=result.next_page_token)
) if result.next_page_token else None
prev_page_url = url_for('explore', **dict(request.args, page=result.prev_page_token)
prev_page_url = url_for('.explore', **dict(request.args, page=result.prev_page_token)
) if result.prev_page_token else None

favourite_programs = g_db().get_hedy_choices()
Expand Down Expand Up @@ -2920,7 +2920,7 @@ def public_user_page(username):
last_achieved = None
certificate_message = safe_format(gettext('see_certificate'), username=username)
next_page_url = url_for(
'public_user_page',
'.public_user_page',
username=username, **dict(request.args,
page=next_page_token)) if next_page_token else None

Expand Down Expand Up @@ -3158,7 +3158,7 @@ def on_offline_mode():
debug = utils.is_debug_mode() and not (is_in_debugger or profile_memory)
if debug:
logger.debug('app starting in debug mode')
app_obj.add_url_rule("/", endpoint="index")

# Threaded option enables multiple instances for multiple user access support
app_obj.run(threaded=True, debug=debug,
port=config['port'], host="0.0.0.0")
Expand Down
12 changes: 6 additions & 6 deletions templates/explore.html
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,14 @@
<div class="w-full mb-4" id="program_filter">
<div class="flex flex-row w-full items-center rounded-lg gap-4">
<custom-select name="adventure" id="explore_page_adventure" data-autosubmit="true" class="w-2/3" data-type="single">
<option hidden
{% if not filtered_adventure %} selected{% endif %}
<option hidden
{% if not filtered_adventure %} selected{% endif %}
value=""
hx-get="/explore?level={{ filtered_level }}"
hx-target="#main_container">{{ _('adventure') }}</option>
{% for adventure_key, name in adventures_names.items() %}
<option hidden
value="{{ adventure_key }}"
<option hidden
value="{{ adventure_key }}"
{% if adventure_key == filtered_adventure %}selected{% endif %}
hx-get="/explore?adventure={{ adventure_key }}&level={{ filtered_level }}"
hx-target="#main_container">{{ name }}</option>
Expand Down Expand Up @@ -55,10 +55,10 @@ <h2 class="text-center z-10 font-bold pb-0 mb-0 text-3xl">{{_('hedy_choice_title
</div>
<div class="flex w-1/3 gap-4 align-self-right">
{% if prev_page_url %}
<a href="{{prev_page_url}}" class="btn green-btn">{{_('previous_page')}}</a>
<a href="{{prev_page_url}}" class="btn green-btn" aria-label="Previous page">{{_('previous_page')}}</a>
{% endif %}
{% if next_page_url %}
<a href="{{next_page_url}}" class="btn green-btn">{{_('next_page')}}</a>
<a href="{{next_page_url}}" class="btn green-btn" aria-label="Next page">{{_('next_page')}}</a>
{% endif %}
</div>
</div>
Expand Down
60 changes: 59 additions & 1 deletion tests/python_html/fixtures/given.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,16 @@
from website.database import Database
from website import auth
import pytest
import utils


def unique_id():
return str(uuid.uuid4())


def now_javascript():
"""Return the current time in milliseconds, or a faked time if configured."""
return utils.timems()


class Given:
Expand All @@ -15,13 +25,14 @@ def __init__(self, client: Client, db: Database):
self.client = client
self.db = db
self.student_ctr = 1
self.teacher_ctr = 1

def a_student_account(self, username=None, password=None, teacher_username=None, language='en'):
"""Create a student account."""
if username is None:
username = f'student{self.student_ctr}'
self.student_ctr += 1
password = password or str(uuid.uuid4())
password = password or unique_id()
student = {
'username': username,
'password': password,
Expand All @@ -31,6 +42,25 @@ def a_student_account(self, username=None, password=None, teacher_username=None,
auth.store_new_student_account(self.db, student, teacher_username)
return {'username': username, 'password': password}

def a_teacher_account(self, username=None, password=None, teacher_username=None, language='en'):
"""Create a student account."""
if username is None:
username = f'student{self.teacher_ctr}'
self.teacher_ctr += 1
password = password or unique_id()
username, hashed, _ = auth.prepare_user_db(username, password)
teacher = {
'username': username,
'password': hashed,
'language': language,
'keyword_language': language,
'is_teacher': 1,
'created': now_javascript(),
'last_login': now_javascript(),
}
self.db.store_user(teacher)
return {'username': username, 'password': password}

def logged_in_as_student(self, username=None):
"""Make sure that we are logged in."""
user = self.a_student_account(username)
Expand All @@ -40,6 +70,34 @@ def logged_in_as_student(self, username=None):
'password': user['password'],
}), content_type='application/json')
assert response.status_code == 200
return user

def logged_in_as_teacher(self, username=None):
"""Make sure that we are logged in as a teacher."""
user = self.a_teacher_account(username)

response = self.client.post('/auth/login', data=json.dumps({
'username': user['username'],
'password': user['password'],
}), content_type='application/json')
assert response.status_code == 200
return user

def some_saved_program(self, username, **kwargs):
"""Save a program for the given user."""
program = {
'id': unique_id(),
'session': username,
'username': username,
'date': now_javascript(),
'lang': 'en',
'level': 1,
'code': 'print TestProgram',
'adventure_name': 'default',
'name': 'TestProgram',
}
program.update(**kwargs)
return self.db.store_program(program)


@pytest.fixture()
Expand Down
23 changes: 23 additions & 0 deletions tests/python_html/test_explore.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
# https://werkzeug.palletsprojects.com/en/stable/test/
from .fixtures.given import Given
from .fixtures.flask import Client
import bs4


def test_explore_page_loads_with_lots_of_programs(client: Client, given: Given):
"""Smoke test of the explore page, if there are enough programs for pagination."""
# GIVEN
user = given.logged_in_as_student()
for _ in range(50):
given.some_saved_program(user['username'], public=1)

# WHEN
response = client.get('/explore')

# THEN - it succeeds, renders a lot of adventures and a next button
soup = bs4.BeautifulSoup(response.data, 'html.parser')
adventures = soup.find_all('div', class_='adventure')
assert len(adventures) > 40

next_page_link = soup.find('a', {'aria-label': 'Next page'})
assert next_page_link
1 change: 0 additions & 1 deletion tests/python_html/test_login.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
# https://werkzeug.palletsprojects.com/en/stable/test/
from .fixtures.given import Given
from .fixtures.flask import Client
import json
import pytest


Expand Down
16 changes: 16 additions & 0 deletions tests/python_html/test_programs.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
# https://werkzeug.palletsprojects.com/en/stable/test/
from .fixtures.given import Given
from .fixtures.flask import Client


def test_programs_page_loads_with_lots_of_programs(client: Client, given: Given):
"""Smoke test of the programs page, if there are enough programs for pagination."""
# GIVEN
user = given.logged_in_as_student()
for _ in range(20):
given.some_saved_program(user['username'])

# WHEN
client.get('/programs')

# THEN - it succeeds
2 changes: 1 addition & 1 deletion website/for_teachers.py
Original file line number Diff line number Diff line change
Expand Up @@ -729,7 +729,7 @@ def public_programs(self, user, class_id, username):
keyword_lang = g.keyword_lang
adventure_names = hedy_content.Adventures(g.lang).get_adventure_names(keyword_lang)

next_page_url = url_for('programs_page', **dict(request.args, page=result.next_page_token)
next_page_url = url_for('app.programs_page', **dict(request.args, page=result.next_page_token)
) if result.next_page_token else None

return render_template(
Expand Down

0 comments on commit bdbbcc2

Please sign in to comment.