Skip to content

Add a group-by-section#85

Merged
IonMich merged 3 commits into
IonMich:mainfrom
AlexRedBlue:group-by-section
Feb 24, 2026
Merged

Add a group-by-section#85
IonMich merged 3 commits into
IonMich:mainfrom
AlexRedBlue:group-by-section

Conversation

@AlexRedBlue

Copy link
Copy Markdown
Contributor

For assignments (quizzes), added an option for the graph to group by section (violin style).

{{ course_pk|json_script:"course_id"}}
<script type="module" src="{% static 'assignments/renderVersionModal.js' %}" defer></script>
<script type="module" src="{% static 'assignments/detail.js' %}" defer></script>
<script type="module" src="{% static 'assignments/detail.js' %}?v=20260222-2" defer></script>

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah, that's nice! I didn't know that this method can force refresh the cache.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Useful if you have run an older version before. I think it was causing an issue with not adding the new button correctly. There's probably a way to do it dynamically too if there are future updates to detail.js for example.

@IonMich IonMich left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR Alex! This is a useful UI element to add in the assignment detail page of the Bootstrap UI. Other than the minor get_section_in_course comment, everything else looks good!

Comment thread submissions/views.py Outdated
question_grades_dict[f'question_{i+1}_grade'] = qg
section = None
if submission.student:
section = submission.student.sections.filter(

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here, can we use get_section_in_course from students/models.py ?

Students who belong in multiple sections of different courses can appear in the database. There are safeguards so that a student always belongs to only one section of a course during initial import, but the role of get_section_in_course is to raise an error if a student accidentally belongs in multiple sections in the same course.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. Could do something like this and catch the error too. Maybe even replace the print with an displayed error.

try:
    section = submission.student.get_section_in_course(assignment.course)
except ValueError as e:
    print(f"Error: {e}")
    section = None

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes that code should work. The print statement is also fine. I believe in the UI we are not properly displaying info anywhere for "unassigned section" / "multiple sections" students, so let's not display an error here. The chart UI in this PR already creates a violin for "Unassigned" students and that will be good enough indication to the TA that something needs fixing.

(I think the course detail page and/or the student detail page could show warnings for such situations, but that's probably separate from this PR.)

@AlexRedBlue
AlexRedBlue requested a review from IonMich February 24, 2026 18:32
@AlexRedBlue

Copy link
Copy Markdown
Contributor Author

I think everything seems okay now. I was seeing if I could dynamically update scripts tags with something like v={hash}, but I would need to do a bit more research into doing that properly.

@AlexRedBlue

Copy link
Copy Markdown
Contributor Author

I think everything seems okay now. I was seeing if I could dynamically update scripts tags with something like v={hash}, but I would need to do a bit more research into doing that properly.

Perhaps all the script tags could be updated like so,

?v={{ ASSET_VERSION }}

Have GitHub Actions set an env var like ASSET_VERSION=${GITHUB_SHA} and use in templates via a context processor.

@IonMich

IonMich commented Feb 24, 2026

Copy link
Copy Markdown
Owner

I think everything seems okay now. I was seeing if I could dynamically update scripts tags with something like v={hash}, but I would need to do a bit more research into doing that properly.

Perhaps all the script tags could be updated like so,

?v={{ ASSET_VERSION }}

Have GitHub Actions set an env var like ASSET_VERSION=${GITHUB_SHA} and use in templates via a context processor.

Thank you Alex! I hadn't thought that this could be reliably automated easily - what you are suggesting would resolve the Ctrl+shift+R that I have to do after each git pull. I believe this asset generation though would create an extra commit for each push/merge to origin/main, right?

In order to avoid that extra commit we could have ASSET_VERSION be calculated dynamically at runtime from the git SHA (something like git rev-parse --short HEAD). But that would probably raise an error in machines without git (e.g. some of the department's Windows machines, where instructor_pilot can still be installed through Github Desktop). Anyway, feel free to create an issue and/or a new PR if you come up with something!

Merging this one!

@IonMich
IonMich merged commit e2abf46 into IonMich:main Feb 24, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants