Skip to content

Commit

Permalink
đŸª² Show unsubmit button to teachers only for programs of their current…
Browse files Browse the repository at this point in the history
… students (#6023)

This PR makes 3 changes:

1. Until now the checkbox for approving student adventures was only shown to the primary teacher. Now the checkbox is also shown to secondary teachers.
2. The unsubmit button is only displayed if the logged-in user is a current teacher (primary or secondary) of the student who submitted the program.
3. The statement that the program is submitted and cannot be altered together with the button to unsubmit have been moved, so that they do not take the upper central part of the screen.

Fixes #5990

**How to test**

Test user permissions

  - Do you agree that a secondary teacher of the class should be able to unsubmit and/or approve submitted programs?
  - A primary teacher can unsubmit the program of their student: log in as a student (student1) enrolled in a class and submit a program. Then log in as the primary teacher (teacher1) and check that you can unsubmit that program, check that you cannot unsubmit a program of the same student which has not been submitted.
  - A secondary teacher can unsubmit the program of their student: log in as a student (student1) enrolled in a class and submit a program. Then log in as the secondary teacher (e.g. teacher4) and check that you can unsubmit that program.
  - A teacher cannot unsubmit the program of a student who is not enrolled in their class: log in as a student (student1) enrolled in a class and submit a program. Then log in a teacher who is not a teacher of the student's class (teacher2) and check that you cannot unsubmit that program. Note that you should be able to see if a program is submitted and when.
  - A student cannot unsubmit the program of another student: log in as another student (student2) and check that you cannot unsubmit that program. Again, you should be able to see if a program is submitted and when

Check whether you agree with the layout. Please note that this page uses too many headings, different styles and overall does not look stylistically coherent. I intentionally made the minimal changes in the layout without changing styling. It would be great to improve its look and feel in the future. I can also revert all layout changes, as I am not sure whether the new layout is better than the old one.

This is how a submitted program looks if you are a teacher of the student:
<img width="1140" alt="Screenshot 2024-12-06 at 12 09 57" src="https://github.com/user-attachments/assets/4e5f04bf-1e7c-47aa-ba1a-b391fe62256b">

And this is how if looks when you are another student or a teacher who is not in the student's class:
<img width="998" alt="Screenshot 2024-12-06 at 12 09 33" src="https://github.com/user-attachments/assets/7aa8d519-f911-4eed-8484-4e84522e37a5">

Note that when you unsubmit a program, the UI should be updated, so that you cannot unsubmit an already unsubmitted program.
  • Loading branch information
boryanagoncharenko authored Dec 11, 2024
1 parent baf2815 commit a97d7c0
Show file tree
Hide file tree
Showing 7 changed files with 99 additions and 54 deletions.
76 changes: 42 additions & 34 deletions app.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
import hedy_translation
import hedyweb
import utils
from dataclasses import dataclass
from hedy_error import get_error_text
from safe_format import safe_format
from config import config
Expand All @@ -48,9 +49,9 @@
cdn, classes, database, for_teachers, s3_logger, parsons,
profile, programs, querylog, quiz, statistics,
translating, tags, surveys, super_teacher, public_adventures, user_activity, feedback)
from website.auth import (current_user, is_admin, is_teacher, is_second_teacher, is_super_teacher, has_public_profile,
login_user_from_token_cookie, requires_login, requires_login_redirect, requires_teacher,
forget_current_user, hide_explore)
from website.auth import (current_user, is_admin, is_teacher, is_second_teacher, is_super_teacher, is_students_teacher,
has_public_profile, login_user_from_token_cookie, requires_login, requires_login_redirect,
requires_teacher, forget_current_user, hide_explore)
from website.log_fetcher import log_fetcher
from website.frontend_types import Adventure, Program, ExtraStory, SaveInfo

Expand Down Expand Up @@ -1226,7 +1227,7 @@ def hour_of_code(level, program_id=None):
loaded_program = None
if program_id:
result = DATABASE.program_by_id(program_id)
if not result or not current_user_allowed_to_see_program(result):
if not result or not get_current_user_program_permissions(result):
return utils.error_page(error=404, ui_message=gettext('no_such_program'))

loaded_program = Program.from_database_row(result)
Expand Down Expand Up @@ -1405,7 +1406,7 @@ def index(level, program_id):
loaded_program = None
if program_id:
result = DATABASE.program_by_id(program_id)
if not result or not current_user_allowed_to_see_program(result):
if not result or not get_current_user_program_permissions(result):
return utils.error_page(error=404, ui_message=gettext('no_such_program'))

loaded_program = Program.from_database_row(result)
Expand Down Expand Up @@ -1636,7 +1637,7 @@ def tryit(level, program_id):
loaded_program = None
if program_id:
result = DATABASE.program_by_id(program_id)
if not result or not current_user_allowed_to_see_program(result):
if not result or not get_current_user_program_permissions(result):
return utils.error_page(error=404, ui_message=gettext('no_such_program'))

loaded_program = Program.from_database_row(result)
Expand Down Expand Up @@ -1874,7 +1875,8 @@ def index_level():
def view_program(user, id):
result = DATABASE.program_by_id(id)

if not result or not current_user_allowed_to_see_program(result):
prog_perms = get_current_user_program_permissions(result)
if not result or not prog_perms:
return utils.error_page(error=404, ui_message=gettext('no_such_program'))

# The program is valid, verify if the creator also have a public profile
Expand Down Expand Up @@ -1917,23 +1919,21 @@ def view_program(user, id):
student_adventure = DATABASE.store_student_adventure(
dict(id=f"{student_adventure_id}", ticked=False, program_id=id))

arguments_dict = {}
arguments_dict['program_id'] = id
arguments_dict['page_title'] = f'{result["name"]} – Hedy'
arguments_dict['level'] = result['level'] # Necessary for running
arguments_dict['initial_adventure'] = dict(result,
editor_contents=code,
)
arguments_dict['editor_readonly'] = True
arguments_dict['student_adventure'] = student_adventure
arguments_dict = {
'program_id': id,
'page_title': f'{result["name"]} – Hedy',
'level': result['level'], # Necessary for running
'initial_adventure': dict(result, editor_contents=code),
'editor_readonly': True,
'student_adventure': student_adventure,
'is_students_teacher': False
}

if "submitted" in result and result['submitted']:
arguments_dict['show_edit_button'] = False
arguments_dict['program_timestamp'] = utils.localized_date_format(result['date'])
else:
arguments_dict['show_edit_button'] = True
is_students_teacher = is_teacher(user)\
and result['username'] in DATABASE.get_teacher_students(user['username'])
arguments_dict['is_students_teacher'] = is_students_teacher

classes = DATABASE.get_student_classes_ids(result['username'])
next_classmate_adventure_id = None
Expand Down Expand Up @@ -1966,14 +1966,16 @@ def view_program(user, id):
if next_program_id:
break

arguments_dict['can_checkoff_program'] = prog_perms.can_checkoff
arguments_dict['can_unsubmit_program'] = prog_perms.can_unsubmit

return render_template("view-program-page.html",
blur_button_available=True,
javascript_page_options=dict(
page='view-program',
lang=g.lang,
level=int(result['level']),
code=code),
is_teacher=user['is_teacher'],
class_id=student_customizations.get('id'),
next_program_id=next_program_id,
next_classmate_program_id=next_classmate_adventure_id,
Expand Down Expand Up @@ -2937,26 +2939,32 @@ def teacher_invitation(code):
return redirect(url)


def current_user_allowed_to_see_program(program):
"""Check if the current user is allowed to see the given program.
@dataclass
class ProgramPermissions:
can_edit: bool
can_checkoff: bool
can_unsubmit: bool


Verify that the program is either public, the current user is the
creator, teacher or the user is admin.
def get_current_user_program_permissions(program):
"""Check if the current user is allowed to view, edit, checkoff or unsubmit the given program.

Verify that the program is either public, the current user is the creator, teacher or the user is admin.
"""
user = current_user()

# These are all easy
if program.get('public'):
return True
if user['username'] == program['username']:
return True
if is_admin(user):
return True
is_current_user_author = program['username'] == user['username']
students_teacher = is_teacher(user) and is_students_teacher(student=program['username'], teacher=user['username'])

can_view = program.get('public') or is_current_user_author or is_admin(user) or students_teacher

if is_teacher(user) and program['username'] in DATABASE.get_teacher_students(user['username']):
return True
if can_view:
can_edit = is_current_user_author
can_checkoff = students_teacher
can_unsubmit = program.get('submitted', False) and (is_admin or students_teacher)
return ProgramPermissions(can_edit, can_checkoff, can_unsubmit)

return False
return None


# *** START SERVER ***
Expand Down
12 changes: 10 additions & 2 deletions static/js/app.ts
Original file line number Diff line number Diff line change
Expand Up @@ -772,10 +772,18 @@ export function submit_program (id: string) {
});
}

export function unsubmit_program (id: string) {
function change_to_unsubmitted () {
$('#unsubmit-program-button').hide();
$('#submitted-program-title').hide();
$('#submitted-program-details').hide();
}

export async function unsubmit_program (id: string, prompt: string) {
await modal.confirmP(prompt);
tryCatchPopup(async () => {
const response = await postJson('/programs/unsubmit', { id });
modal.notifySuccess(response.message);
change_to_unsubmitted();
});
}

Expand Down Expand Up @@ -1975,4 +1983,4 @@ export function goToLevel(level: any) {

export function emptyEditor() {
theGlobalEditor.contents = ""
}
}
9 changes: 8 additions & 1 deletion static/js/appbundle.js
Original file line number Diff line number Diff line change
Expand Up @@ -121128,10 +121128,17 @@ def note_with_error(value, err):
change_to_submitted(id2);
});
}
function unsubmit_program(id2) {
function change_to_unsubmitted() {
$("#unsubmit-program-button").hide();
$("#submitted-program-title").hide();
$("#submitted-program-details").hide();
}
async function unsubmit_program(id2, prompt) {
await modal.confirmP(prompt);
tryCatchPopup(async () => {
const response = await postJson("/programs/unsubmit", { id: id2 });
modal.notifySuccess(response.message);
change_to_unsubmitted();
});
}
async function set_explore_favourite(id2, favourite) {
Expand Down
4 changes: 2 additions & 2 deletions static/js/appbundle.js.map

Large diffs are not rendered by default.

38 changes: 23 additions & 15 deletions templates/view-program-page.html
Original file line number Diff line number Diff line change
@@ -1,21 +1,9 @@
{% extends "layout.html" %}
{% block full_width_content %}
<div style="height: 30px;"></div>
<div tabindex="0" class="flex-grow px-8 common-page-container">
{% if initial_adventure.submitted %}
<div class="text-center">
<h2>{{_('submitted_header')}}</h2>
<h3 class="inline-block mt-0">{{_('submission_time')}}: {{program_timestamp}}</h3>
</div>
{% if is_teacher %}
<div class="text-center">
<button class="green-btn" data-cy="unsubmit_btn" onclick="hedyApp.modal.confirm ('{{_('unsubmit_warning')}}', function () {hedyApp.unsubmit_program ('{{initial_adventure.id}}', {{ loop_index }})})">{{_('unsubmit_program')}}</button>
</div>
{% endif %}
{% endif %}
<div class="flex flex-row gap-6 items-center">
<h1>{{initial_adventure.name}}</h1>
{% if is_students_teacher %}
{% if can_checkoff_program %}
<input type="checkbox"
id="adventure_checkbox"
class="student_adventure_checkbox text-green-700 text-center p-3 text-lg cursor-pointer self-center {% if student_adventure.ticked %}fa-solid fa-check{% endif %} "
Expand All @@ -25,6 +13,13 @@ <h1>{{initial_adventure.name}}</h1>
{% if student_adventure.ticked %}checked{% endif %}
_="on click toggle .fa-solid .fa-check on me"
>

{% if can_unsubmit_program %}
<div class="text-center">
<button id="unsubmit-program-button" class="green-btn" data-cy="unsubmit_btn" onclick="hedyApp.unsubmit_program('{{initial_adventure.id}}', '{{_('unsubmit_warning')}}')">{{_('unsubmit_program')}}</button>
</div>
{% endif %}

<div class="flex-1"></div>
{% if next_program_id %}
<a class="green-btn" href="/hedy/{{ next_program_id }}/view">
Expand All @@ -41,10 +36,23 @@ <h1>{{initial_adventure.name}}</h1>
{% endif %}
{% endif %}
</div>
<h2 class="p-0 m-0">{{_('level_title')}} {{level}}</h2>
<div>{{_('by')}}
<div class="flex flex-row gap-6 items-center">
<h2 class="p-0 m-0">{{_('level_title')}} {{level}}</h2>
{% if initial_adventure.submitted %}
<div class="flex-1"></div>
<h2 id="submitted-program-title" class="p-0 m-0">{{_('submitted_header')}}</h2>
{% endif %}
</div>
<div class="flex flex-row gap-6 items-center">
{% if initial_adventure.submitted %}
<div>{{_('by')}}
<a {% if initial_adventure.public_profile %}onclick="window.open('/user/{{initial_adventure.username}}', '_self')"{% endif %} class="{% if initial_adventure.public_profile %}cursor-pointer{% else %}text-black{% endif %} no-underline">{{ initial_adventure.username }}</a>
</div>
<div class="flex-1"></div>
<h3 id="submitted-program-details" class="p-0 m-0">{{_('submission_time')}} {{program_timestamp}}</h3>
{% endif %}
</div>

{% include "incl/editor-and-output.html" %}
</div>
{% endblock %}
4 changes: 4 additions & 0 deletions website/auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,10 @@ def is_super_teacher(user):
return bool(user.get("is_super_teacher", False))


def is_students_teacher(student, teacher):
return teacher in g.db.get_student_teachers(student)


def has_public_profile(user):
if 'username' not in user or user.get('username') == '':
return False
Expand Down
10 changes: 10 additions & 0 deletions website/database.py
Original file line number Diff line number Diff line change
Expand Up @@ -756,6 +756,16 @@ def get_teacher_students(self, username):
students.append(student)
return students

def get_student_teachers(self, username):
"""Return a list of the main and all secondary teachers of a student."""
teachers = []
for class_id in self.get_student_classes_ids(username):
class_ = self.get_class(class_id)
teachers.append(class_["teacher"])
sec_teachers = [t['username'] for t in class_.get('second_teachers', []) if t.get('role', '') == 'teacher']
teachers.extend(sec_teachers)
return teachers

def get_adventure(self, adventure_id):
return self.adventures.get({"id": adventure_id})

Expand Down

0 comments on commit a97d7c0

Please sign in to comment.