diff --git a/CHANGELOG.md b/CHANGELOG.md index 749362169..c86a95764 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,4 +1,6 @@ ## Unreleased + +- added stat cards to the group view - Added stat cards to the student view - Added stat cards to organization view - Added a clear icon to search input and removed dead student performance view diff --git a/app/controllers/groups_controller.rb b/app/controllers/groups_controller.rb index 88b78434a..abdea9af9 100644 --- a/app/controllers/groups_controller.rb +++ b/app/controllers/groups_controller.rb @@ -1,3 +1,4 @@ +# rubocop:disable Metrics/ClassLength class GroupsController < HtmlController include Pagy::Method @@ -22,6 +23,9 @@ def show lesson_url: lesson_path(Lesson.find_by(id: summary.lesson_id)) } end + @nr_of_active_students = active_student_count + @current_average_score = @group_summaries.last&.dig(:average_mark) + populate_skill_growth end def new @@ -107,6 +111,43 @@ def confirm_enrollments private + def active_student_count + @group.enrollments + .active + .joins(:student) + .where(students: { deleted_at: nil }) + .distinct + .count(:student_id) + end + + def populate_skill_growth + growths = average_growth_per_skill + @most_improved_skill = growths.min_by { |g| [-g[:growth], g[:skill_name], g[:skill_id]] } + @least_improved_skill = growths.min_by { |g| [g[:growth], g[:skill_name], g[:skill_id]] } + end + + def average_growth_per_skill + deltas_by_skill = Hash.new { |hash, key| hash[key] = [] } + marks_by_student_and_skill.each do |(_student_id, skill_id, skill_name), marks| + next if marks.size < 2 + + deltas_by_skill[[skill_id, skill_name]] << (marks.last - marks.first) + end + + deltas_by_skill.map { |(skill_id, skill_name), deltas| { skill_id:, skill_name:, growth: deltas.sum.to_f / deltas.size } } + end + + def marks_by_student_and_skill + marks = Hash.new { |hash, key| hash[key] = [] } + Grade.joins(:lesson, :skill) + .where(deleted_at: nil) + .where(lessons: { group_id: @group.id, deleted_at: nil }) + .order('lessons.date ASC') + .pluck(:student_id, 'skills.id', 'skills.skill_name', :mark) + .each { |student_id, skill_id, skill_name, mark| marks[[student_id, skill_id, skill_name]] << mark } + marks + end + def group_params params.require(:group).permit :group_name, :mlid, :chapter_id end @@ -121,3 +162,4 @@ def new_params params.permit :chapter_id end end +# rubocop:enable Metrics/ClassLength diff --git a/app/models/enrollment.rb b/app/models/enrollment.rb index 75c3fd52d..aa2312a30 100644 --- a/app/models/enrollment.rb +++ b/app/models/enrollment.rb @@ -28,6 +28,9 @@ class Enrollment < ApplicationRecord scope :by_student, ->(student_id) { where student_id: } scope :by_group, ->(group_id) { where group_id: } + # Enrollments that are open at `as_of`: started on/before it and not yet ended (inactive_since is + # exclusive, matching Student#active_enrollment? and Student.unenrolled_for_organization). + scope :active, ->(as_of = Time.zone.now) { where('active_since <= ? AND (inactive_since IS NULL OR inactive_since > ?)', as_of, as_of) } validates :active_since, presence: true validates :inactive_since, comparison: { greater_than: :active_since, message: I18n.t(:enrollment_end_before_start) }, allow_nil: true diff --git a/app/views/groups/show.html.erb b/app/views/groups/show.html.erb index 8fc43e8b0..a74096265 100644 --- a/app/views/groups/show.html.erb +++ b/app/views/groups/show.html.erb @@ -26,13 +26,29 @@ end %> <% end %>
-
- <%= render CommonComponents::Card.new(title: t(:average_performance_for_last_30_lessons).capitalize) do |card| %> - <% card.with_card_content do %> -
- <% end %> - <% end %> +
+
+ <%= render CommonComponents::Card.new(title: t(:average_performance_for_last_30_lessons).capitalize) do |card| %> + <% card.with_card_content do %> +
+ <% end %> + <% end %>
+
+
+ <%= render CommonComponents::StatCards.new( + label: t(:overview).capitalize, + columns: 2, + stats: [ + { title: t(:nr_of_active_students), value: @nr_of_active_students }, + { title: t(:current_average_score_for_group), value: @current_average_score&.round(2) || t(:student_not_graded) }, + { title: t(:most_improved_skill), value: @most_improved_skill&.dig(:skill_name) || t(:student_not_graded) }, + { title: t(:least_improved_skill), value: @least_improved_skill&.dig(:skill_name) || t(:student_not_graded) } + ] + ) %> +
+
+
<%= render GroupEnrolledStudentsComponent.new(students: @group.students, group: @group, students_with_invalid_grades: @students_with_invalid_grades) %> diff --git a/config/locales/en.yml b/config/locales/en.yml index e858f488b..4ebd73a06 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -409,6 +409,7 @@ en: nr_of_users_with_role: Number of users with a role within the Organization nr_of_lessons_present: Number of lessons present total_average_score: Total average score + current_average_score_for_group: Current average score for group most_improved_skill: Most improved skill least_improved_skill: Least improved skill overview: Overview diff --git a/spec/controllers/groups_controller_spec.rb b/spec/controllers/groups_controller_spec.rb index b1bdd601f..62af29e75 100644 --- a/spec/controllers/groups_controller_spec.rb +++ b/spec/controllers/groups_controller_spec.rb @@ -133,6 +133,155 @@ it { should respond_with 200 } end + + context 'skill growth' do + before :each do + subject = create :subject_with_skills, skill_names: %w[Memorization Grit], organization: @group.chapter.organization + create :lesson_with_grades, group: @group, subject:, date: 2.days.ago, + student_grades: { @student1.id => { 'Memorization' => 1, 'Grit' => 3 } } + create :lesson_with_grades, group: @group, subject:, date: 1.day.ago, + student_grades: { @student1.id => { 'Memorization' => 3, 'Grit' => 6 } } + + get :show, params: { id: @group.id } + end + + it 'assigns the most and least improved skill based on first vs last average' do + expect(assigns(:most_improved_skill)[:skill_name]).to eq 'Grit' + expect(assigns(:least_improved_skill)[:skill_name]).to eq 'Memorization' + end + end + + context 'skill growth across multiple students' do + before :each do + subject = create :subject_with_skills, skill_names: %w[Discipline Grit], organization: @group.chapter.organization + @student3 = create :enrolled_student, organization: @group.chapter.organization, groups: [@group] + @student4 = create :enrolled_student, organization: @group.chapter.organization, groups: [@group] + @student5 = create :enrolled_student, organization: @group.chapter.organization, groups: [@group] + + # 3 students each improve a little in Discipline; 2 students each improve a lot in Grit. + create :lesson_with_grades, group: @group, subject:, date: 2.days.ago, + student_grades: { + @student1.id => { 'Discipline' => 2 }, @student2.id => { 'Discipline' => 2 }, @student3.id => { 'Discipline' => 2 }, + @student4.id => { 'Grit' => 1 }, @student5.id => { 'Grit' => 1 } + } + create :lesson_with_grades, group: @group, subject:, date: 1.day.ago, + student_grades: { + @student1.id => { 'Discipline' => 3 }, @student2.id => { 'Discipline' => 3 }, @student3.id => { 'Discipline' => 3 }, + @student4.id => { 'Grit' => 7 }, @student5.id => { 'Grit' => 7 } + } + + get :show, params: { id: @group.id } + end + + it 'picks the skill with the biggest average improvement, not the one most students individually led in' do + expect(assigns(:most_improved_skill)[:skill_name]).to eq 'Grit' + expect(assigns(:least_improved_skill)[:skill_name]).to eq 'Discipline' + end + end + + context 'skill graded only once' do + before :each do + subject = create :subject_with_skills, skill_names: %w[Memorization Grit Creativity], organization: @group.chapter.organization + create :lesson_with_grades, group: @group, subject:, date: 2.days.ago, + student_grades: { @student1.id => { 'Memorization' => 1, 'Grit' => 3 } } + create :lesson_with_grades, group: @group, subject:, date: 1.day.ago, + student_grades: { @student1.id => { 'Memorization' => 3, 'Grit' => 6, 'Creativity' => 5 } } + + get :show, params: { id: @group.id } + end + + it 'ignores skills with only a single grade so they cannot win most/least improved' do + expect(assigns(:most_improved_skill)[:skill_name]).to eq 'Grit' + expect(assigns(:least_improved_skill)[:skill_name]).to eq 'Memorization' + end + end + + context 'skills sharing a name across subjects' do + before :each do + subject_a = create :subject_with_skills, skill_names: %w[Discipline Grit], organization: @group.chapter.organization + subject_b = create :subject_with_skills, skill_names: %w[Discipline], organization: @group.chapter.organization + + create :lesson_with_grades, group: @group, subject: subject_a, date: 2.days.ago, + student_grades: { @student1.id => { 'Discipline' => 1, 'Grit' => 2 } } + create :lesson_with_grades, group: @group, subject: subject_a, date: 1.day.ago, + student_grades: { @student1.id => { 'Discipline' => 7, 'Grit' => 5 } } + create :lesson_with_grades, group: @group, subject: subject_b, date: 2.days.ago, + student_grades: { @student1.id => { 'Discipline' => 7 } } + create :lesson_with_grades, group: @group, subject: subject_b, date: 1.day.ago, + student_grades: { @student1.id => { 'Discipline' => 1 } } + + get :show, params: { id: @group.id } + end + + it 'keeps same-named skills from different subjects separate when computing growth' do + expect(assigns(:most_improved_skill)[:skill_name]).to eq 'Discipline' + expect(assigns(:least_improved_skill)[:skill_name]).to eq 'Discipline' + end + end + + context 'when several skills tie on growth' do + before :each do + subject = create :subject_with_skills, skill_names: %w[Zeta Alpha Mu], organization: @group.chapter.organization + create :lesson_with_grades, group: @group, subject:, date: 2.days.ago, + student_grades: { @student1.id => { 'Zeta' => 1, 'Alpha' => 1, 'Mu' => 3 } } + create :lesson_with_grades, group: @group, subject:, date: 1.day.ago, + student_grades: { @student1.id => { 'Zeta' => 3, 'Alpha' => 3, 'Mu' => 2 } } + + get :show, params: { id: @group.id } + end + + it 'breaks the most-improved tie by skill name, not database order' do + expect(assigns(:most_improved_skill)[:skill_name]).to eq 'Alpha' + end + + it 'still picks the genuinely least improved skill' do + expect(assigns(:least_improved_skill)[:skill_name]).to eq 'Mu' + end + end + + context 'group statistics' do + before :each do + # @student1 and @student2 have open enrollments; these two must be excluded from the active count: + # one whose enrollment already ended, and one whose enrollment has not started yet. + inactive_student = create :student, organization: @group.chapter.organization + create :enrollment, student: inactive_student, group: @group, active_since: 1.year.ago.to_date, inactive_since: 1.month.ago.to_date + not_yet_active_student = create :student, organization: @group.chapter.organization + create :enrollment, student: not_yet_active_student, group: @group, active_since: 1.month.from_now.to_date + + subject = create :subject_with_skills, skill_names: %w[Memorization Grit], organization: @group.chapter.organization + create :lesson_with_grades, group: @group, subject:, date: 2.days.ago, + student_grades: { @student1.id => { 'Memorization' => 1, 'Grit' => 3 } } + create :lesson_with_grades, group: @group, subject:, date: 1.day.ago, + student_grades: { @student1.id => { 'Memorization' => 3, 'Grit' => 6 } } + + get :show, params: { id: @group.id } + end + + it 'counts only currently-active, non-deleted enrolled students' do + expect(assigns(:nr_of_active_students)).to eq 2 + end + + it 'exposes the most recent lesson average as the current score' do + expect(assigns(:current_average_score)).to be_a(Numeric) + end + end + + context 'when no lessons have been graded yet' do + before :each do + @ungraded_group = create :group + get :show, params: { id: @ungraded_group.id } + end + + it 'assigns nil for the skill growth statistics' do + expect(assigns(:most_improved_skill)).to be_nil + expect(assigns(:least_improved_skill)).to be_nil + end + + it 'assigns zero active students and a nil current score' do + expect(assigns(:nr_of_active_students)).to eq 0 + expect(assigns(:current_average_score)).to be_nil + end + end end describe '#edit' do