openwebwork / webwork2

Course management front end for WeBWorK
http://webwork.maa.org/wiki/Main_Page
Other
141 stars 164 forks source link

Filter student progress for single set by section/recitation. #2320

Closed somiaj closed 5 months ago

somiaj commented 5 months ago

The main goal was to add the ability to filter student progress for a single set by section/recitation. This is done by first updating Utils::FilterRecords::getFiltersForClass to be able to limit which types of filters to include. Then use the Utils::FilterRecords methods to both get and filter the student records to be displayed. In this process update the Stats filters to also use Utils::FilterRecords methods instead of their own method.

Translation was also added to the visible strings in Utils::FilterRecords::getFiltersForClass.

somiaj commented 5 months ago

The file is now used by two templates in templates/ContentGenerator/Instructor/Stats and one template in templates/ContentGenerator/Instructor/StudentProgress so I thought a more generic location would be preferred. I will move it back unless you have a better place to put this.

somiaj commented 5 months ago

@Alex-Jordan it occurred to me there might have been a reason the strings in templates/HTML/ScrollingRecordList via the methods in lib/WeBWorK/Utils/FilterRecords.pm were not translated. Just double checking I'm not over looking anything adding translations to the visible strings.

drgrice1 commented 5 months ago

The file is now used by two templates in templates/ContentGenerator/Instructor/Stats and one template in templates/ContentGenerator/Instructor/StudentProgress so I thought a more generic location would be preferred. I will move it back unless you have a better place to put this.

I realized that after I made the comment. This is not the first template file shared by Stats and StudentProgress. The student_stats.html.ep template is in fact shared by Stats, StudentProgress, and Grades. The templates/ContentGenerator/Instructor/Stats/siblings.html.ep is also used by both Stats and StudentProgress.

The point is that the templates in templates/HTML are for the modules in lib/WeBWorK/HTML. All templates directly used by ContentGenerator modules should be somewhere inside the templates/ContentGenerator directory. An argument could be made for a shared subdirectory there perhaps.

@Alex-Jordan it occurred to me there might have been a reason the strings in templates/HTML/ScrollingRecordList via the methods in lib/WeBWorK/Utils/FilterRecords.pm were not translated. Just double checking I'm not over looking anything adding translations to the visible strings.

Those strings should be translated. It was just an oversight earlier. Previously those couldn't be translated because the controller object wasn't available, and when that was added they should have been translated.

pstaabp commented 5 months ago

Overall, this is a nice addition. It will be useful when I teach multiple sections of a class again.

My only small quibble is the boldness of the pulldown button. I changed the class of the button to btn-secondary and looks better to me, but not a huge concern.

somiaj commented 5 months ago

@pstaabp I have no opinion on what the button should look like. I just want to point out that it currently matches the filters seen on other pages, such as when viewing a problem in a homework set or test while acting as a student. This button matches those.

somiaj commented 5 months ago

Though here btn-secondary is bright orange, so sticks out more than btn-primary. Anyways, I think consistency is probably best, so if we change the button look here, should we change it elsewhere?

pstaabp commented 5 months ago

@somiaj I'm fine keeping it btn-primary. The default theme, which I'm running has the primary a bold blue and secondary a gray. And yes I see that the "action" buttons on the sets and accounts manager are also primary.