From bdbbcc20a24ec0b311573119d46d08f324f0eb99 Mon Sep 17 00:00:00 2001 From: Rico Hermans Date: Sat, 28 Dec 2024 00:35:07 +0100 Subject: [PATCH] =?UTF-8?q?=F0=9F=AA=B2=20Fix=20pagination=20links=20(#607?= =?UTF-8?q?5)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- app.py | 12 +++--- templates/explore.html | 12 +++--- tests/python_html/fixtures/given.py | 60 ++++++++++++++++++++++++++++- tests/python_html/test_explore.py | 23 +++++++++++ tests/python_html/test_login.py | 1 - tests/python_html/test_programs.py | 16 ++++++++ website/for_teachers.py | 2 +- 7 files changed, 111 insertions(+), 15 deletions(-) create mode 100644 tests/python_html/test_explore.py create mode 100644 tests/python_html/test_programs.py diff --git a/app.py b/app.py index 7329bd4c35b..77ef56ee475 100644 --- a/app.py +++ b/app.py @@ -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( @@ -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() @@ -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 @@ -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") diff --git a/templates/explore.html b/templates/explore.html index 9af173c2bd8..4c55b7e41dd 100644 --- a/templates/explore.html +++ b/templates/explore.html @@ -4,14 +4,14 @@
- {% for adventure_key, name in adventures_names.items() %} - @@ -55,10 +55,10 @@

{{_('hedy_choice_title

{% if prev_page_url %} - {{_('previous_page')}} + {{_('previous_page')}} {% endif %} {% if next_page_url %} - {{_('next_page')}} + {{_('next_page')}} {% endif %}
diff --git a/tests/python_html/fixtures/given.py b/tests/python_html/fixtures/given.py index 9f24ee3c5f8..ddbbfd5311d 100644 --- a/tests/python_html/fixtures/given.py +++ b/tests/python_html/fixtures/given.py @@ -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: @@ -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, @@ -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) @@ -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() diff --git a/tests/python_html/test_explore.py b/tests/python_html/test_explore.py new file mode 100644 index 00000000000..033c584ca00 --- /dev/null +++ b/tests/python_html/test_explore.py @@ -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 diff --git a/tests/python_html/test_login.py b/tests/python_html/test_login.py index 38f5870d01f..ae15a0e0393 100644 --- a/tests/python_html/test_login.py +++ b/tests/python_html/test_login.py @@ -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 diff --git a/tests/python_html/test_programs.py b/tests/python_html/test_programs.py new file mode 100644 index 00000000000..a9ad267bd20 --- /dev/null +++ b/tests/python_html/test_programs.py @@ -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 diff --git a/website/for_teachers.py b/website/for_teachers.py index d8d1a301457..b84de674a23 100644 --- a/website/for_teachers.py +++ b/website/for_teachers.py @@ -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(