From 15f293e9fbcd0513a2a820e5e21d36e8810d7a9d Mon Sep 17 00:00:00 2001
From: Kevin Rukundo <kevinrukundo1@gmail.com>
Date: Thu, 23 Jul 2026 20:11:20 +0200
Subject: [PATCH 1/5] feat(2629): add stat cards to group view

---
 app/controllers/groups_controller.rb       | 14 +++++++++++
 app/views/groups/show.html.erb             | 23 ++++++++++++-----
 config/database.yml                        |  1 +
 config/locales/en.yml                      |  2 --
 spec/controllers/groups_controller_spec.rb | 29 ++++++++++++++++++++++
 5 files changed, 61 insertions(+), 8 deletions(-)

diff --git a/app/controllers/groups_controller.rb b/app/controllers/groups_controller.rb
index 88b78434a..6604f122b 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,7 @@ def show
         lesson_url: lesson_path(Lesson.find_by(id: summary.lesson_id))
       }
     end
+    populate_skill_growth
   end
 
   def new
@@ -107,6 +109,17 @@ def confirm_enrollments
 
   private
 
+  def populate_skill_growth
+    marks_by_skill = Hash.new { |hash, key| hash[key] = [] }
+    PerformancePerGroupPerSkillPerLesson.where(group_id: @group.id).order(date: :asc).each do |performance|
+      marks_by_skill[performance.skill_name] << performance.mark
+    end
+
+    growths = marks_by_skill.map { |skill_name, marks| { skill_name:, growth: marks.last - marks.first } }
+    @most_improved_skill = growths.max_by { |g| g[:growth] }
+    @least_improved_skill = growths.min_by { |g| g[:growth] }
+  end
+
   def group_params
     params.require(:group).permit :group_name, :mlid, :chapter_id
   end
@@ -121,3 +134,4 @@ def new_params
     params.permit :chapter_id
   end
 end
+# rubocop:enable Metrics/ClassLength
diff --git a/app/views/groups/show.html.erb b/app/views/groups/show.html.erb
index 8fc43e8b0..440ecaea8 100644
--- a/app/views/groups/show.html.erb
+++ b/app/views/groups/show.html.erb
@@ -26,13 +26,24 @@ end %>
 <% end %>
 
 <section>
-   <div class="w-1/2">
-       <%= render CommonComponents::Card.new(title: t(:average_performance_for_last_30_lessons).capitalize) do |card| %>
-             <% card.with_card_content do %>
-               <div id="group-chart" class="ct-chart ct-octave bg-white full-width"></div>
-             <% end %>
-       <% end %>
+  <div class="flex gap-4">
+    <div class="w-1/2">
+        <%= render CommonComponents::Card.new(title: t(:average_performance_for_last_30_lessons).capitalize) do |card| %>
+              <% card.with_card_content do %>
+                <div id="group-chart" class="ct-chart ct-octave bg-white full-width"></div>
+              <% end %>
+        <% end %>
     </div>
+    <div class="w-1/2">
+      <%# TODO: these two StatCards still show placeholder values, not real data - see student/organization show pages for the pattern to wire them up %>
+      <dl class="mt-14 grid grid-cols-2 gap-5">
+        <%= render CommonComponents::StatCard.new(title: t(:nr_of_active_students), value: '18') %>
+        <%= render CommonComponents::StatCard.new(title: t(:current_average_score_for_group), value: '3.8') %>
+        <%= render CommonComponents::StatCard.new(title: t(:most_improved_skill), value: @most_improved_skill&.dig(:skill_name) || t(:student_not_graded)) %>
+        <%= render CommonComponents::StatCard.new(title: t(:least_improved_skill), value: @least_improved_skill&.dig(:skill_name) || t(:student_not_graded)) %>
+      </dl>
+    </div>
+  </div>
 </section>
 <turbo-frame id="students-table">
   <%= render GroupEnrolledStudentsComponent.new(students: @group.students, group: @group, students_with_invalid_grades: @students_with_invalid_grades) %>
diff --git a/config/database.yml b/config/database.yml
index a253baef9..4fcf77802 100644
--- a/config/database.yml
+++ b/config/database.yml
@@ -3,6 +3,7 @@ default: &default
   pool: 5
   timeout: 5000
   host: <%= ENV['DATABASE_HOST'] || 'localhost' %>
+  port: 5433
   username: <%= ENV['DATABASE_USER'] || 'tracker' %>
   password: <%= ENV['DATABASE_PASSWORD'] || 'tracker' %>
 
diff --git a/config/locales/en.yml b/config/locales/en.yml
index 5b42d215c..2b7856ca1 100644
--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@@ -407,8 +407,6 @@ en:
   nr_of_active_groups: Number of active Groups
   nr_of_active_students: Number of active Students
   nr_of_users_with_role: Number of users with a role within the organization
-  best_skill: Best skill
-  worst_skill: Worst skill
   nr_of_lessons_present: Number of lessons present
   total_average_score: Total average score
   most_improved_skill: Most improved skill
diff --git a/spec/controllers/groups_controller_spec.rb b/spec/controllers/groups_controller_spec.rb
index b1bdd601f..578d36ecd 100644
--- a/spec/controllers/groups_controller_spec.rb
+++ b/spec/controllers/groups_controller_spec.rb
@@ -133,6 +133,35 @@
 
       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 '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
+    end
   end
 
   describe '#edit' do

From bfeaa4f4f064eb33a7382ab5c82925ed833c3c93 Mon Sep 17 00:00:00 2001
From: Kevin Rukundo <kevinrukundo1@gmail.com>
Date: Thu, 23 Jul 2026 20:17:07 +0200
Subject: [PATCH 2/5] feat(2629): add group view stat cards changes to
 CHANGELOG

---
 CHANGELOG.md        | 1 +
 config/database.yml | 1 -
 2 files changed, 1 insertion(+), 1 deletion(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 8b0cff42c..993d5683c 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,4 +1,5 @@
 ## Unreleased
+- added stat cards to the group view
 - added stat cards to the student view
 - Added a clear icon to search input and removed dead student performance view
 - Added a message to search results when no results are found
diff --git a/config/database.yml b/config/database.yml
index 4fcf77802..a253baef9 100644
--- a/config/database.yml
+++ b/config/database.yml
@@ -3,7 +3,6 @@ default: &default
   pool: 5
   timeout: 5000
   host: <%= ENV['DATABASE_HOST'] || 'localhost' %>
-  port: 5433
   username: <%= ENV['DATABASE_USER'] || 'tracker' %>
   password: <%= ENV['DATABASE_PASSWORD'] || 'tracker' %>
 

From 59200c79613aa2694e4b14de80d5138e2994f87a Mon Sep 17 00:00:00 2001
From: Kevin Rukundo <kevinrukundo1@gmail.com>
Date: Thu, 6 Aug 2026 07:22:37 +0200
Subject: [PATCH 3/5] feat(2629): change group stat cards calculation logic

---
 app/controllers/groups_controller.rb       | 29 +++++++++++++++++-----
 spec/controllers/groups_controller_spec.rb | 28 +++++++++++++++++++++
 2 files changed, 51 insertions(+), 6 deletions(-)

diff --git a/app/controllers/groups_controller.rb b/app/controllers/groups_controller.rb
index 6604f122b..b2c300950 100644
--- a/app/controllers/groups_controller.rb
+++ b/app/controllers/groups_controller.rb
@@ -110,16 +110,33 @@ def confirm_enrollments
   private
 
   def populate_skill_growth
-    marks_by_skill = Hash.new { |hash, key| hash[key] = [] }
-    PerformancePerGroupPerSkillPerLesson.where(group_id: @group.id).order(date: :asc).each do |performance|
-      marks_by_skill[performance.skill_name] << performance.mark
-    end
-
-    growths = marks_by_skill.map { |skill_name, marks| { skill_name:, growth: marks.last - marks.first } }
+    growths = average_growth_per_skill
     @most_improved_skill = growths.max_by { |g| g[:growth] }
     @least_improved_skill = growths.min_by { |g| g[:growth] }
   end
 
+  # For each skill, averages every student's own (last mark - first mark) so the skill with the
+  # largest real improvement wins, rather than whichever skill the most students happened to lead in.
+  def average_growth_per_skill
+    deltas_by_skill = Hash.new { |hash, key| hash[key] = [] }
+    marks_by_student_and_skill.each do |(_student_id, skill_name), marks|
+      deltas_by_skill[skill_name] << (marks.last - marks.first)
+    end
+
+    deltas_by_skill.map { |skill_name, deltas| { 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.skill_name', :mark)
+         .each { |student_id, skill_name, mark| marks[[student_id, skill_name]] << mark }
+    marks
+  end
+
   def group_params
     params.require(:group).permit :group_name, :mlid, :chapter_id
   end
diff --git a/spec/controllers/groups_controller_spec.rb b/spec/controllers/groups_controller_spec.rb
index 578d36ecd..de9e0bdf0 100644
--- a/spec/controllers/groups_controller_spec.rb
+++ b/spec/controllers/groups_controller_spec.rb
@@ -151,6 +151,34 @@
       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 'when no lessons have been graded yet' do
       before :each do
         @ungraded_group = create :group

From 79a38a998b9bd6fdd506089a76fe6b0cfbba98fc Mon Sep 17 00:00:00 2001
From: Kevin Rukundo <kevinrukundo1@gmail.com>
Date: Thu, 6 Aug 2026 23:10:53 +0200
Subject: [PATCH 4/5] feat(2629): wire group stat cards to real data and
 stabilize skill growth calculation

---
 app/controllers/groups_controller.rb       | 29 ++++---
 app/models/enrollment.rb                   |  3 +
 app/views/groups/show.html.erb             | 19 +++--
 config/locales/en.yml                      |  1 +
 spec/controllers/groups_controller_spec.rb | 92 ++++++++++++++++++++++
 5 files changed, 128 insertions(+), 16 deletions(-)

diff --git a/app/controllers/groups_controller.rb b/app/controllers/groups_controller.rb
index b2c300950..abdea9af9 100644
--- a/app/controllers/groups_controller.rb
+++ b/app/controllers/groups_controller.rb
@@ -23,6 +23,8 @@ 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
 
@@ -109,21 +111,30 @@ 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.max_by { |g| g[:growth] }
-    @least_improved_skill = growths.min_by { |g| g[:growth] }
+    @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
 
-  # For each skill, averages every student's own (last mark - first mark) so the skill with the
-  # largest real improvement wins, rather than whichever skill the most students happened to lead in.
   def average_growth_per_skill
     deltas_by_skill = Hash.new { |hash, key| hash[key] = [] }
-    marks_by_student_and_skill.each do |(_student_id, skill_name), marks|
-      deltas_by_skill[skill_name] << (marks.last - marks.first)
+    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_name, deltas| { skill_name:, growth: deltas.sum.to_f / deltas.size } }
+    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
@@ -132,8 +143,8 @@ def marks_by_student_and_skill
          .where(deleted_at: nil)
          .where(lessons: { group_id: @group.id, deleted_at: nil })
          .order('lessons.date ASC')
-         .pluck(:student_id, 'skills.skill_name', :mark)
-         .each { |student_id, skill_name, mark| marks[[student_id, skill_name]] << mark }
+         .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
 
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 440ecaea8..a74096265 100644
--- a/app/views/groups/show.html.erb
+++ b/app/views/groups/show.html.erb
@@ -35,13 +35,18 @@ end %>
         <% end %>
     </div>
     <div class="w-1/2">
-      <%# TODO: these two StatCards still show placeholder values, not real data - see student/organization show pages for the pattern to wire them up %>
-      <dl class="mt-14 grid grid-cols-2 gap-5">
-        <%= render CommonComponents::StatCard.new(title: t(:nr_of_active_students), value: '18') %>
-        <%= render CommonComponents::StatCard.new(title: t(:current_average_score_for_group), value: '3.8') %>
-        <%= render CommonComponents::StatCard.new(title: t(:most_improved_skill), value: @most_improved_skill&.dig(:skill_name) || t(:student_not_graded)) %>
-        <%= render CommonComponents::StatCard.new(title: t(:least_improved_skill), value: @least_improved_skill&.dig(:skill_name) || t(:student_not_graded)) %>
-      </dl>
+      <div class="mt-6">
+        <%= 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) }
+          ]
+        ) %>
+      </div>
     </div>
   </div>
 </section>
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 de9e0bdf0..62af29e75 100644
--- a/spec/controllers/groups_controller_spec.rb
+++ b/spec/controllers/groups_controller_spec.rb
@@ -179,6 +179,93 @@
       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
@@ -189,6 +276,11 @@
         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
 

From 8307784825250a395ffb67df6e9ace78245c59d5 Mon Sep 17 00:00:00 2001
From: Kevin Rukundo <kevinrukundo1@gmail.com>
Date: Thu, 13 Aug 2026 10:59:17 +0200
Subject: [PATCH 5/5] feat(2629): fix performance issue with group view by
 limiting grades to active students - Capitalize the first letter of ChangeLog
 entry for consistency - remove AI comments

---
 CHANGELOG.md                               |   2 +-
 app/controllers/groups_controller.rb       |  23 +--
 app/helpers/application_helper.rb          |   2 +-
 app/models/enrollment.rb                   |   2 -
 app/models/group.rb                        |  24 +++
 app/views/groups/show.html.erb             |   8 +-
 app/views/students/show.html.erb           |   6 +-
 config/locales/en.yml                      |   2 +-
 spec/controllers/groups_controller_spec.rb | 174 ++++-----------------
 spec/features/group_features_spec.rb       |  32 ++++
 spec/models/group_spec.rb                  |  29 ++++
 11 files changed, 131 insertions(+), 173 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index c86a95764..408faca04 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,6 +1,6 @@
 ## Unreleased
 
-- added stat cards to the group view
+- 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 abdea9af9..c9939b77e 100644
--- a/app/controllers/groups_controller.rb
+++ b/app/controllers/groups_controller.rb
@@ -23,7 +23,6 @@ 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
@@ -111,15 +110,6 @@ 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]] }
@@ -139,12 +129,13 @@ def average_growth_per_skill
 
   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 }
+    active_student_ids = @group.active_students.pluck(:id)
+    @group.valid_grades
+          .where(student_id: active_student_ids)
+          .joins(:skill)
+          .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
 
diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb
index 766702f5d..025891d49 100644
--- a/app/helpers/application_helper.rb
+++ b/app/helpers/application_helper.rb
@@ -63,7 +63,7 @@ def name_or_email(user)
   end
 
   def lesson_student_average_grade
-    t :student_not_graded
+    t :not_graded
   end
 
   def student_mini_thumbnail(student)
diff --git a/app/models/enrollment.rb b/app/models/enrollment.rb
index aa2312a30..e3e5cef8f 100644
--- a/app/models/enrollment.rb
+++ b/app/models/enrollment.rb
@@ -28,8 +28,6 @@ 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
diff --git a/app/models/group.rb b/app/models/group.rb
index 4389da56a..abc07a013 100644
--- a/app/models/group.rb
+++ b/app/models/group.rb
@@ -88,4 +88,28 @@ def students_with_grades_outside_enrollment
               SQL
             )
   end
+
+  def active_students(as_of: Time.zone.today)
+    Student
+      .joins(:enrollments)
+      .merge(enrollments.active(as_of))
+      .where(enrollments: { group_id: id }, students: { deleted_at: nil })
+      .distinct
+  end
+
+  def valid_grades
+    Grade.joins(:lesson)
+         .where(deleted_at: nil, lessons: { group_id: id, deleted_at: nil })
+         .where(
+           <<~SQL.squish
+             EXISTS (
+               SELECT 1
+               FROM enrollments e
+               WHERE e.student_id = grades.student_id
+                 AND e.group_id = lessons.group_id
+                 AND lessons.date BETWEEN e.active_since AND COALESCE(e.inactive_since, 'infinity')
+             )
+           SQL
+         )
+  end
 end
diff --git a/app/views/groups/show.html.erb b/app/views/groups/show.html.erb
index a74096265..87d646437 100644
--- a/app/views/groups/show.html.erb
+++ b/app/views/groups/show.html.erb
@@ -40,10 +40,10 @@ end %>
           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) }
+            { title: t(:nr_of_active_students), value: @group.active_students.count },
+            { title: t(:current_average_score_for_group), value: @current_average_score&.round(2) || t(:not_graded) },
+            { title: t(:most_improved_skill), value: @most_improved_skill&.dig(:skill_name) || t(:not_graded) },
+            { title: t(:least_improved_skill), value: @least_improved_skill&.dig(:skill_name) || t(:not_graded) }
           ]
         ) %>
       </div>
diff --git a/app/views/students/show.html.erb b/app/views/students/show.html.erb
index eb4c21838..f218057d7 100644
--- a/app/views/students/show.html.erb
+++ b/app/views/students/show.html.erb
@@ -32,9 +32,9 @@ end %>
             label: t(:student_statistics),
             stats: [
               { title: t(:nr_of_lessons_present), value: @nr_of_lessons_present },
-              { title: t(:total_average_score), value: @total_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) }
+              { title: t(:total_average_score), value: @total_average_score&.round(2) || t(:not_graded) },
+              { title: t(:most_improved_skill), value: @most_improved_skill&.dig(:skill_name) || t(:not_graded) },
+              { title: t(:least_improved_skill), value: @least_improved_skill&.dig(:skill_name) || t(:not_graded) }
             ]
           ) %>
         </div>
diff --git a/config/locales/en.yml b/config/locales/en.yml
index 4ebd73a06..54f9eb574 100644
--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@@ -228,7 +228,7 @@ en:
   student_grades: Student Grades
   student_graded: Student graded.
   student_graded_text: Student %{student} graded.
-  student_not_graded: Not Graded
+  not_graded: Not Graded
   add_grade: Add Grade
   graded_vs_students_in_group: Graded / Students in Group
   back_to_subjects: Back to All Subjects
diff --git a/spec/controllers/groups_controller_spec.rb b/spec/controllers/groups_controller_spec.rb
index 62af29e75..46458f4e6 100644
--- a/spec/controllers/groups_controller_spec.rb
+++ b/spec/controllers/groups_controller_spec.rb
@@ -134,152 +134,36 @@
       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
+      context 'when the group has graded lessons' 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 current average score and the most/least improved skill' do
+          expect(assigns(:current_average_score)).to be_a(Numeric)
+          expect(assigns(:most_improved_skill)[:skill_name]).to eq 'Grit'
+          expect(assigns(:least_improved_skill)[:skill_name]).to eq 'Memorization'
+        end
+      end
+
+      context 'when the group has no graded lessons' do
+        before :each do
+          @ungraded_group = create :group
+          get :show, params: { id: @ungraded_group.id }
+        end
+
+        it 'assigns nil for the current average score and the skill growth statistics' do
+          expect(assigns(:current_average_score)).to be_nil
+          expect(assigns(:most_improved_skill)).to be_nil
+          expect(assigns(:least_improved_skill)).to be_nil
+        end
       end
     end
   end
diff --git a/spec/features/group_features_spec.rb b/spec/features/group_features_spec.rb
index 401a5827c..96fe5bd4a 100644
--- a/spec/features/group_features_spec.rb
+++ b/spec/features/group_features_spec.rb
@@ -239,4 +239,36 @@
       expect(page).to have_selector('span.group > .tooltip', visible: :all, text: expected_alert_text)
     end
   end
+
+  describe 'Group statistics' do
+    it 'shows active student count, current average, and most/least improved skill for the current roster only' do
+      group = create :group, group_name: 'Growth Stats Group'
+      subject = create :subject_with_skills, skill_names: %w[Memorization Grit], organization: group.chapter.organization
+
+      active_student = create :student, organization: group.chapter.organization
+      create :enrollment, student: active_student, group: group, active_since: 1.month.ago.to_date
+      second_active_student = create :student, organization: group.chapter.organization
+      create :enrollment, student: second_active_student, group: group, active_since: 1.month.ago.to_date
+
+      # This student already left the group. Their grades were valid while they were enrolled, and show a huge
+      # improvement in Memorization -- if they were wrongly counted, that swing would hijack "most improved".
+      left_student = create :student, organization: group.chapter.organization
+      create :enrollment, student: left_student, group: group, active_since: 1.year.ago.to_date, inactive_since: 1.month.ago.to_date
+      create :lesson_with_grades, group: group, subject:, date: 11.months.ago, student_grades: { left_student.id => { 'Memorization' => 1 } }
+      create :lesson_with_grades, group: group, subject:, date: 10.months.ago, student_grades: { left_student.id => { 'Memorization' => 7 } }
+
+      create :lesson_with_grades, group: group, subject:, date: 2.days.ago,
+                                  student_grades: { active_student.id => { 'Memorization' => 1 }, second_active_student.id => { 'Grit' => 1 } }
+      create :lesson_with_grades, group: group, subject:, date: 1.day.ago,
+                                  student_grades: { active_student.id => { 'Memorization' => 2 }, second_active_student.id => { 'Grit' => 3 } }
+
+      visit "/groups/#{group.id}"
+
+      stat_values = all('dd.tracking-tight').map(&:text)
+      expect(stat_values[0]).to eq '2' # active students: left_student excluded
+      expect(stat_values[1]).to eq '2.5' # average of the two active students' marks on the most recent lesson
+      expect(stat_values[2]).to eq 'Grit' # most improved, among active students only (delta +2, vs Memorization's +1)
+      expect(stat_values[3]).to eq 'Memorization' # would lose to Memorization's +6 if left_student were wrongly counted
+    end
+  end
 end
diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb
index ad46bc2ae..91fcbcb52 100644
--- a/spec/models/group_spec.rb
+++ b/spec/models/group_spec.rb
@@ -139,5 +139,34 @@
         expect(result).not_to include @students[4]
       end
     end
+
+    describe '#active_students' do
+      before :each do
+        @group = create :group
+        @currently_enrolled = create :student, organization: @group.chapter.organization
+        create :enrollment, group: @group, student: @currently_enrolled, active_since: 1.week.ago
+
+        @already_left = create :student, organization: @group.chapter.organization
+        create :enrollment, group: @group, student: @already_left, active_since: 1.year.ago.to_date, inactive_since: 1.month.ago.to_date
+
+        @not_yet_started = create :student, organization: @group.chapter.organization
+        create :enrollment, group: @group, student: @not_yet_started, active_since: 1.month.from_now.to_date
+
+        @deleted_student = create :student, organization: @group.chapter.organization, deleted_at: Time.zone.now
+        create :enrollment, group: @group, student: @deleted_student, active_since: 1.week.ago
+      end
+
+      it 'returns only students with an open enrollment covering the given date who are not deleted' do
+        result = @group.active_students
+
+        expect(result).to contain_exactly @currently_enrolled
+      end
+
+      it 'accepts an as_of date to check enrollment at a different point in time' do
+        result = @group.active_students(as_of: 6.months.ago)
+
+        expect(result).to contain_exactly @already_left
+      end
+    end
   end
 end