refactor: remove 'menu' parameter (hedyorg#1356)

All calls to `render_template()` used to include a `menu=` parameter, to render the entries of the main menu (in every language). It was an annoying requirement to pass in the menu everywhere, and unnecessary because the menu could just be a global variable in the template. Add a template global `main_menu_entries()` which returns the same as what `render_main_menu()` used to do. Instead, `menu` is now a boolean that controls whether or not the menu should be rendered at all, and the default is `True` so in most cases you don't need to pass anything: you only need to pass `menu=False` in cases where you don't want the menu rendered, which is only for embedded frames like for the quiz. Relates to hedyorg#1179. Co-authored-by: Felienne <[email protected]>
RMerbis · Nov 21, 2021 · 42ab264 · 42ab264
1 parent 4a0bac0
commit 42ab264
Show file tree

Hide file tree

Showing 7 changed files with 59 additions and 57 deletions.
diff --git a/app.py b/app.py
@@ -541,10 +541,10 @@ def programs_page(request):
     from_user = request.args.get('user') or None
     if from_user and not is_admin(user):
         if not is_teacher(user):
-            return utils.page_403 (TRANSLATIONS, render_main_menu('hedy'), username, g.lang, TRANSLATIONS.get_translations (g.lang, 'ui').get ('not_teacher'))
+            return utils.page_403 (TRANSLATIONS, username, g.lang, TRANSLATIONS.get_translations (g.lang, 'ui').get ('not_teacher'))
         students = DATABASE.get_teacher_students(username)
         if from_user not in students:
-            return utils.page_403 (TRANSLATIONS, render_main_menu('hedy'), username, g.lang, TRANSLATIONS.get_translations (g.lang, 'ui').get ('not_enrolled'))
+            return utils.page_403 (TRANSLATIONS, username, g.lang, TRANSLATIONS.get_translations (g.lang, 'ui').get ('not_enrolled'))
 
     texts=TRANSLATIONS.get_translations(g.lang, 'Programs')
     ui=TRANSLATIONS.get_translations(g.lang, 'ui')
@@ -567,12 +567,12 @@ def programs_page(request):
 
         programs.append({'id': item['id'], 'code': item['code'], 'date': texts['ago-1'] + ' ' + str(date) + ' ' + measure + ' ' + texts['ago-2'], 'level': item['level'], 'name': item['name'], 'adventure_name': item.get('adventure_name'), 'public': item.get('public')})
 
-    return render_template('programs.html', menu=render_main_menu('programs'), texts=texts, ui=ui, auth=TRANSLATIONS.get_translations(g.lang, 'Auth'), programs=programs, current_page='programs', from_user=from_user, adventures=adventures)
+    return render_template('programs.html', texts=texts, ui=ui, auth=TRANSLATIONS.get_translations(g.lang, 'Auth'), programs=programs, current_page='programs', from_user=from_user, adventures=adventures)
 
 @app.route('/quiz/start/<int:level>', methods=['GET'])
 def get_quiz_start(level):
     if not config.get('quiz-enabled') and g.lang != 'nl':
-        return utils.page_404 (TRANSLATIONS, render_main_menu('adventures'), current_user()['username'], g.lang, 'Hedy quiz disabled!')
+        return utils.page_404 (TRANSLATIONS, current_user()['username'], g.lang, 'Hedy quiz disabled!', menu=False)
     else:
         g.prefix = '/hedy'
 
@@ -583,7 +583,7 @@ def get_quiz_start(level):
         session['total_score'] = 0
         session['correct_answer'] = 0
 
-        return render_template('startquiz.html', level=level, next_assignment=1, menu=render_main_menu('adventures'),
+        return render_template('startquiz.html', level=level, next_assignment=1,
                                auth=TRANSLATIONS.get_translations(g.lang, 'Auth'))
 
 # Quiz mode
@@ -592,7 +592,7 @@ def get_quiz_start(level):
 @app.route('/quiz/quiz_questions/<int:level_source>/<int:question_nr>/<int:attempt>', methods=['GET'])
 def get_quiz(level_source, question_nr, attempt):
     if not config.get('quiz-enabled') and g.lang != 'nl':
-        return utils.page_404 (TRANSLATIONS, render_main_menu('adventures'), current_user()['username'], g.lang, 'Hedy quiz disabled!')
+        return utils.page_404 (TRANSLATIONS, current_user()['username'], g.lang, 'Hedy quiz disabled!', menu=False)
 
     # If we don't have an attempt ID yet, redirect to the start page
     if not session.get('quiz-attempt-id'):
@@ -634,15 +634,14 @@ def get_quiz(level_source, question_nr, attempt):
                            correct=session.get('correct_answer'),
                            attempt = attempt,
                            is_last_attempt=attempt == quiz.MAX_ATTEMPTS,
-                           menu=render_main_menu('adventures'),
                            auth=TRANSLATIONS.get_translations(g.lang, 'Auth'))
 
 
 @app.route('/quiz/finished/<int:level>', methods=['GET'])
 def quiz_finished(level):
     """Results page at the end of the quiz."""
     if not config.get('quiz-enabled') and g.lang != 'nl':
-        return utils.page_404 (TRANSLATIONS, render_main_menu('adventures'), current_user() ['username'], g.lang, 'Hedy quiz disabled!')
+        return utils.page_404 (TRANSLATIONS, current_user() ['username'], g.lang, 'Hedy quiz disabled!', menu=False)
 
     # Reading the yaml file
     quiz_data = quiz.quiz_data_file_for(level)
@@ -654,7 +653,6 @@ def quiz_finished(level):
 
     return render_template('endquiz.html', correct=session.get('correct_answer', 0),
                            total_score= round(session.get('total_score', 0) / quiz.max_score(quiz_data) * 100),
-                           menu=render_main_menu('adventures'),
                            quiz=quiz_data, level=int(level) + 1, questions=quiz_data['questions'],
                            next_assignment=1,
                            auth=TRANSLATIONS.get_translations (g.lang, 'Auth'))
@@ -760,7 +758,6 @@ def quiz_feedback(level_source, question_nr):
                            wrong_answer_hint=wrong_answer_hint,
                            index_option=index_option,
                            correct_option=correct_option,
-                           menu=render_main_menu('adventures'),
                            auth=TRANSLATIONS.data[g.lang]['Auth'])
 
 # routing to index.html
@@ -776,9 +773,9 @@ def index(level, step):
         try:
             g.level = level = int(level)
         except:
-            return utils.page_404 (TRANSLATIONS, render_main_menu('hedy'), current_user()['username'], g.lang, TRANSLATIONS.get_translations (g.lang, 'ui').get ('no_such_level'))
+            return utils.page_404 (TRANSLATIONS, current_user()['username'], g.lang, TRANSLATIONS.get_translations (g.lang, 'ui').get ('no_such_level'))
     else:
-        return utils.page_404 (TRANSLATIONS, render_main_menu('hedy'), current_user()['username'], g.lang, TRANSLATIONS.get_translations (g.lang, 'ui').get ('no_such_level'))
+        return utils.page_404 (TRANSLATIONS, current_user()['username'], g.lang, TRANSLATIONS.get_translations (g.lang, 'ui').get ('no_such_level'))
 
     g.prefix = '/hedy'
 
@@ -789,12 +786,12 @@ def index(level, step):
     if step and isinstance(step, str) and len(step) > 2:
         result = DATABASE.program_by_id(step)
         if not result:
-            return utils.page_404 (TRANSLATIONS, render_main_menu('hedy'), current_user()['username'], g.lang, TRANSLATIONS.get_translations (g.lang, 'ui').get ('no_such_program'))
+            return utils.page_404 (TRANSLATIONS, current_user()['username'], g.lang, TRANSLATIONS.get_translations (g.lang, 'ui').get ('no_such_program'))
         # If the program is not public, allow only the owner of the program, the admin user and the teacher users to access the program
         user = current_user()
         public_program = 'public' in result and result['public']
         if not public_program and user['username'] != result['username'] and not is_admin(user) and not is_teacher(user):
-            return utils.page_404 (TRANSLATIONS, render_main_menu('hedy'), current_user()['username'], g.lang, TRANSLATIONS.get_translations (g.lang, 'ui').get ('no_such_program'))
+            return utils.page_404 (TRANSLATIONS, current_user()['username'], g.lang, TRANSLATIONS.get_translations (g.lang, 'ui').get ('no_such_program'))
         loaded_program = {'code': result['code'], 'name': result['name'], 'adventure_name': result.get('adventure_name')}
         if 'adventure_name' in result:
             adventure_name = result['adventure_name']
@@ -803,15 +800,14 @@ def index(level, step):
     level_defaults_for_lang = LEVEL_DEFAULTS[g.lang]
 
     if level not in level_defaults_for_lang.levels or restrictions['hide_level']:
-        return utils.page_404 (TRANSLATIONS, render_main_menu('hedy'), current_user()['username'], g.lang, TRANSLATIONS.get_translations (g.lang, 'ui').get ('no_such_level'))
+        return utils.page_404 (TRANSLATIONS, current_user()['username'], g.lang, TRANSLATIONS.get_translations (g.lang, 'ui').get ('no_such_level'))
     defaults = level_defaults_for_lang.get_defaults_for_level(level)
     max_level = level_defaults_for_lang.max_level()
 
     return hedyweb.render_code_editor_with_tabs(
         level_defaults=defaults,
         max_level=max_level,
         level_number=level,
-        menu=render_main_menu('hedy'),
         translations=TRANSLATIONS,
         version=version(),
         adventures=adventures,
@@ -866,7 +862,7 @@ def view_program(id):
 
     result = DATABASE.program_by_id(id)
     if not result:
-        return utils.page_404 (TRANSLATIONS, render_main_menu('hedy'), user['username'], g.lang, TRANSLATIONS.get_translations (g.lang, 'ui').get ('no_such_program'))
+        return utils.page_404 (TRANSLATIONS, user['username'], g.lang, TRANSLATIONS.get_translations (g.lang, 'ui').get ('no_such_program'))
 
     # If we asked for a specific language, use that, otherwise use the language
     # of the program's author.
@@ -884,7 +880,7 @@ def view_program(id):
 
     # Everything below this line has nothing to do with this page and it's silly
     # that every page needs to put in so much effort to re-set it
-    arguments_dict['menu'] = render_main_menu('view')
+    arguments_dict['menu'] = True
     arguments_dict['auth'] = TRANSLATIONS.get_translations(lang, 'Auth')
     arguments_dict['username'] = user.get('username', None)
     arguments_dict['is_teacher'] = is_teacher(user)
@@ -914,13 +910,13 @@ def client_messages():
 
 @app.errorhandler(404)
 def not_found(exception):
-    return utils.page_404 (TRANSLATIONS, render_main_menu('adventures'), current_user()['username'], g.lang, TRANSLATIONS.get_translations (g.lang, 'ui').get ('page_not_found'))
+    return utils.page_404 (TRANSLATIONS, current_user()['username'], g.lang, TRANSLATIONS.get_translations (g.lang, 'ui').get ('page_not_found'))
 
 @app.errorhandler(500)
 def internal_error(exception):
     import traceback
     print(traceback.format_exc())
-    return utils.page_500 (TRANSLATIONS, render_main_menu('hedy'), current_user()['username'], g.lang)
+    return utils.page_500 (TRANSLATIONS, current_user()['username'], g.lang)
 
 @app.route('/index.html')
 @app.route('/')
@@ -933,7 +929,7 @@ def main_page(page):
         abort(404)
 
     if page in['signup', 'login', 'my-profile', 'recover', 'reset', 'admin']:
-        return auth_templates(page, g.lang, render_main_menu(page), request)
+        return auth_templates(page, g.lang, request)
 
     if page == 'programs':
         return programs_page(request)
@@ -950,20 +946,19 @@ def main_page(page):
     front_matter, markdown = split_markdown_front_matter(contents)
 
     user = current_user()
-    menu = render_main_menu(page)
     if page == 'for-teachers':
         if is_teacher(user):
             welcome_teacher = session.get('welcome-teacher') or False
             session['welcome-teacher'] = False
             teacher_classes =[] if not current_user()['username'] else DATABASE.get_teacher_classes(current_user()['username'], True)
-            return render_template('for-teachers.html', sections=split_teacher_docs(contents), menu=menu,
+            return render_template('for-teachers.html', sections=split_teacher_docs(contents),
                                    auth=TRANSLATIONS.get_translations(g.lang, 'Auth'), teacher_classes=teacher_classes,
                                    welcome_teacher=welcome_teacher, **front_matter)
         else:
-            return utils.page_403 (TRANSLATIONS, render_main_menu('hedy'), current_user()['username'], g.lang, TRANSLATIONS.get_translations (g.lang, 'ui').get ('not_teacher'))
+            return utils.page_403 (TRANSLATIONS, current_user()['username'], g.lang, TRANSLATIONS.get_translations (g.lang, 'ui').get ('not_teacher'))
 
 
-    return render_template('main-page.html', mkd=markdown, menu=menu, auth=TRANSLATIONS.get_translations(g.lang, 'Auth'), **front_matter)
+    return render_template('main-page.html', mkd=markdown, auth=TRANSLATIONS.get_translations(g.lang, 'Auth'), **front_matter)
 
 
 def session_id():
@@ -979,6 +974,17 @@ def session_id():
 def current_language():
     return make_lang_obj(g.lang)
 
+@app.template_global()
+def main_menu_entries():
+    """Return the entries that make up the main menu.
+
+    Calls render_main_menu() to do it, and assume the first part of the current
+    request's path is the "current page".
+    """
+    # path starts with '/', in case of empty call it 'start'
+    first_path_component = request.path[1:].split('/')[0] or 'start'
+    return render_main_menu(first_path_component)
+
 @app.template_filter()
 def nl2br(x):
     """Turn newlines into <br>"""
@@ -1232,11 +1238,10 @@ def teacher_invitation(code):
     lang = g.lang
 
     if os.getenv('TEACHER_INVITE_CODE') != code:
-        return utils.page_404(TRANSLATIONS, render_main_menu('invite'), user['username'], lang,
+        return utils.page_404(TRANSLATIONS, user['username'], lang,
                               TRANSLATIONS.get_translations(g.lang, 'ui').get('invalid_teacher_invitation_code'))
     if not user['username']:
-        return render_template('teacher-invitation.html', auth=TRANSLATIONS.get_translations(lang, 'Auth'),
-                               menu=render_main_menu('invite'))
+        return render_template('teacher-invitation.html', auth=TRANSLATIONS.get_translations(lang, 'Auth'))
 
     update_is_teacher(user)
 
@@ -1279,4 +1284,4 @@ def on_server_start():
     # Threaded option enables multiple instances for multiple user access support
     app.run(threaded=True, debug=not is_in_debugger, port=config['port'], host="0.0.0.0")
 
-    # See `Procfile` for how the server is started on Heroku.
+    # See `Procfile` for how the server is started on Heroku.
diff --git a/hedyweb.py b/hedyweb.py
@@ -31,11 +31,11 @@ def get_translations(self, language, section):
     return d
 
 
-def render_code_editor_with_tabs(level_defaults, max_level, level_number, menu, translations, version, loaded_program, adventures, restrictions, adventure_name):
+def render_code_editor_with_tabs(level_defaults, max_level, level_number, translations, version, loaded_program, adventures, restrictions, adventure_name):
   user = current_user()
 
   if not level_defaults:
-    return utils.page_404 (translations, menu, user['username'], g.lang, translations.get_translations (g.lang, 'ui').get ('no_such_level'))
+    return utils.page_404 (translations, user['username'], g.lang, translations.get_translations (g.lang, 'ui').get ('no_such_level'))
 
 
   arguments_dict = {}
@@ -49,7 +49,7 @@ def render_code_editor_with_tabs(level_defaults, max_level, level_number, menu,
   arguments_dict['example_programs'] = restrictions['example_programs']
   arguments_dict['hide_prev_level'] = restrictions['hide_prev_level']
   arguments_dict['hide_next_level'] = restrictions['hide_next_level']
-  arguments_dict['menu'] = menu
+  arguments_dict['menu'] = True
   arguments_dict['latest'] = version
   arguments_dict['selected_page'] = 'code'
   arguments_dict['page_title'] = f'Level {level_number} – Hedy'

diff --git a/templates/incl-menubar.html b/templates/incl-menubar.html
@@ -6,19 +6,19 @@
     </div>
 
     {% block menu %}
-    {% for item in menu %}
+    {% for item in main_menu_entries() %}
       {% if item.short_name != 'for-teachers' or is_teacher %}
         <a class="menubar-btn {% if item.selected %}border-{{item.accent_color}}{% else %}border-transparent{% endif %} " href="{{localize_link (item.href)}}">
           {{ item.caption }}
         </a>
       {% endif %}
     {% endfor %}
     {% if username %}
-      <a class="menubar-btn border-{% if current_page == 'my-profile' %}white{% else %}transparent{% endif %}" href="{{localize_link ('/my-profile')}}">{{auth.profile}}</a>
-      <a class="menubar-btn border-{% if current_page == 'programs' %}white{% else %}transparent{% endif %}" href="{{localize_link ('/programs')}}">{{auth.program_header}}</a>
+      <a class="menubar-btn {% if current_page == 'my-profile' %}border-white{% else %}border-transparent{% endif %}" href="{{localize_link ('/my-profile')}}">{{auth.profile}}</a>
+      <a class="menubar-btn {% if current_page == 'programs' %}border-white{% else %}border-transparent{% endif %}" href="{{localize_link ('/programs')}}">{{auth.program_header}}</a>
     {% endif %}
     {% if not username %}
-      <a class="menubar-btn border-{% if current_page == 'login' %}white{% else %}transparent{% endif %}" href="{{localize_link ('/login')}}">{{auth.login}}</a>
+      <a class="menubar-btn {% if current_page == 'login' %}border-white{% else %}border-transparent{% endif %}" href="{{localize_link ('/login')}}">{{auth.login}}</a>
     {% endif %}
     {% endblock %}
     <div class="dropdown inline-block relative ml-auto py-2 z-40">

diff --git a/templates/layout.html b/templates/layout.html
@@ -38,7 +38,7 @@
        </div>
     </div>
     <div class="lg:container mx-auto bg-gray-100 min-h-screen shadow-md flex flex-col">
-      {% if menu %}
+      {% if menu != False %}
       {% filter indent(width=6) %}{% include "incl-menubar.html" %}{% endfilter %}
       {% endif %}
       {# Can't reindent this as it may contain preformatted code blocks whose indentation is important. #}

diff --git a/utils.py b/utils.py
@@ -199,17 +199,17 @@ def markdown_to_html_tags(markdown):
     soup = BeautifulSoup(_html, 'html.parser')
     return soup.find_all()
 
-def page_403(translations, menu, lang, username, *page_error):
+def page_403(translations, lang, username, *page_error, menu=True):
     if page_error:
         page_error = page_error[0]
     return render_template("403.html", menu=menu, username=username, auth=translations.get_translations(lang, 'Auth'), ui=translations.get_translations(lang, 'ui'), page_error=page_error or ''), 403
 
-def page_404(translations, menu, lang, username, *page_error):
+def page_404(translations, lang, username, *page_error, menu=True):
     if page_error:
         page_error = page_error[0]
     return render_template("404.html", menu=menu, username=username, auth=translations.get_translations(lang, 'Auth'), ui=translations.get_translations(lang, 'ui'), page_error=page_error or ''), 404
 
-def page_500(translations, menu, lang, username, *page_error):
+def page_500(translations, lang, username, *page_error, menu=True):
     if page_error:
         page_error = page_error[0]
     return render_template("500.html", menu=menu, username=username, auth=translations.get_translations(lang, 'Auth'), ui=translations.get_translations(lang, 'ui'), page_error=page_error or ''), 500

diff --git a/website/auth.py b/website/auth.py
@@ -602,11 +602,11 @@ def send_email_template(template, email, link):
 
     send_email(email, subject, body_plain, body_html)
 
-def auth_templates(page, lang, menu, request):
+def auth_templates(page, lang, request):
     if page == 'my-profile':
-        return render_template('profile.html', auth=TRANSLATIONS.get_translations(lang, 'Auth'), menu=menu, current_page='my-profile')
+        return render_template('profile.html', auth=TRANSLATIONS.get_translations(lang, 'Auth'), current_page='my-profile')
     if page in['signup', 'login', 'recover', 'reset']:
-        return render_template(page + '.html',  auth=TRANSLATIONS.get_translations(lang, 'Auth'), menu=menu, is_teacher=False, current_page='login')
+        return render_template(page + '.html',  auth=TRANSLATIONS.get_translations(lang, 'Auth'), is_teacher=False, current_page='login')
     if page == 'admin':
         if not is_testing_request(request) and not is_admin(current_user()):
             return 'unauthorized', 403