Merge pull request #4247 from dodona-edu/feature/show-class-progress-…

…for-teachers Show class progress on series homepage cards for teachers
dodona-edu · Jan 11, 2023 · fd3a899 · fd3a899
2 parents 6dcfe87 + 4a14dc8
commit fd3a899
Show file tree

Hide file tree

Showing 6 changed files with 183 additions and 14 deletions.
diff --git a/app/assets/stylesheets/bootstrap_variable_overrides.css.scss b/app/assets/stylesheets/bootstrap_variable_overrides.css.scss
@@ -67,3 +67,6 @@ $offcanvas-border-width: 0;
 $btn-padding-x: 24px;
 $btn-border-radius: 20px;
 $btn-border-width: 0;
+
+// Tooltip
+$tooltip-max-width: 400px;
diff --git a/app/models/series.rb b/app/models/series.rb
@@ -164,6 +164,15 @@ def started_activity_count(user)
     activities.count { |a| a.started_for?(user, self) }
   end
 
+  def users_started
+    activity_statuses.where(started: true, user: course.subscribed_members).distinct.count(:user_id)
+  end
+
+  def users_completed
+    # first count is used for the group by and ignored, second is used for the actual count of users
+    activity_statuses.where(accepted: true, user: course.subscribed_members).group('user_id').having('COUNT(*) = ?', activities_count).count.count
+  end
+
   def scoresheet
     users = course.subscribed_members.order_by_status_in_course_and_name 'ASC'
 

diff --git a/app/views/pages/clickable_homepage_cards/_series.html.erb b/app/views/pages/clickable_homepage_cards/_series.html.erb
@@ -1,6 +1,7 @@
+<% teacher_mode = current_user.course_admin?(series.course) %>
 <% text ||= false %>
 <% card_class = nil
-   card_class = series.completed_before_deadline?(current_user) ? "colored-success" : "colored-danger" if series.deadline.present? %>
+   card_class = series.completed_before_deadline?(current_user) ? "colored-success" : "colored-danger" if series.deadline.present? && !teacher_mode %>
 <%= link_to course_path(series.course, series: series, anchor: series.anchor) do %>
   <div class="card card-supporting-text action-card <%= card_class %>">
     <div class="card-subtitle">
@@ -11,8 +12,15 @@
           <i class="mdi mdi-36 <%= deadline_icon series.completed_before_deadline?(current_user), series.deadline %>"></i>
         <% end %>
       </div>
-      <h4 class="ellipsis-overflow" title="<%= series.name %>">
-        <%= series.name %><br>
+      <h4 class="ellipsis-overflow">
+        <div class="spread-line">
+          <span class="ellipsis-overflow" title="<%= series.name %>">
+            <%= series.name %>
+          </span>
+          <% if teacher_mode && !text %>
+            <i class="mdi mdi-school mdi-18" title="<%= t ".teacher_mode_info" %>"></i>
+          <% end %>
+        </div>
         <div class="spread-line" >
           <span class="ellipsis-overflow" style="flex: 2">
             <small  title="<%= series.course.name %>">
@@ -26,15 +34,26 @@
       </h4>
     </div>
     <% unless text %>
-      <%= render partial: 'progress_chart', locals: {
-        total: series.activity_count,
-        tried: series.started_activity_count(current_user),
-        correct: series.completed_activity_count(current_user),
-        info_wrong: 'pages.clickable_homepage_cards.series.progress_chart_info_wrong',
-        info_not_started: 'pages.clickable_homepage_cards.series.progress_chart_info_not_started',
-        info_correct: 'pages.clickable_homepage_cards.series.progress_chart_info_correct',
-        width: '100%',
-      } %>
+      <% if teacher_mode %>
+        <%= render partial: 'progress_chart', locals: {
+          total: series.course.subscribed_members.count,
+          tried: series.users_started,
+          correct: series.users_completed,
+          info_tried: 'pages.clickable_homepage_cards.series.teacher_progress_chart_info_tried',
+          info_correct: 'pages.clickable_homepage_cards.series.teacher_progress_chart_info_correct',
+          width: '100%',
+        } %>
+      <% else %>
+        <%= render partial: 'progress_chart', locals: {
+          total: series.activity_count,
+          tried: series.started_activity_count(current_user),
+          correct: series.completed_activity_count(current_user),
+          info_wrong: 'pages.clickable_homepage_cards.series.progress_chart_info_wrong',
+          info_not_started: 'pages.clickable_homepage_cards.series.progress_chart_info_not_started',
+          info_correct: 'pages.clickable_homepage_cards.series.progress_chart_info_correct',
+          width: '100%',
+        } %>
+      <% end %>
     <% else %>
         <small class="ellipsis-overflow">
             <%= text %>

diff --git a/config/locales/views/pages/en.yml b/config/locales/views/pages/en.yml
@@ -40,6 +40,13 @@ en:
       next_series: "Or start the next series."
       course: "Continue working on this course."
       series:
+        teacher_mode_info: "Class progress is only shown to teachers. Students only see their own progress."
+        teacher_progress_chart_info_correct:
+          one: "One student has solved all exercises correctly."
+          other: "%{count} students have solved all exercises correctly."
+        teacher_progress_chart_info_tried:
+          one: "One student has started this series."
+          other: "%{count} students have started this series."
         progress_chart_info_correct:
           one: "One exercise solved correctly."
           other: "%{count} exercises solved correctly."

diff --git a/config/locales/views/pages/nl.yml b/config/locales/views/pages/nl.yml
@@ -39,9 +39,14 @@ nl:
       next_series_success: "De vorige reeks was correct, start met deze reeks."
       next_series: "Of start met de volgende reeks."
       course: "Werk verder aan deze cursus."
-
       series:
-        jump_back_in_info: "Je hebt de vorige reeks afgewerkt, start met deze reeks."
+        teacher_mode_info: "De voortgang van de klas wordt enkel getoond aan leraren. Studenten zien enkel hun eigen voortgang."
+        teacher_progress_chart_info_correct:
+          one: "Eén student heeft alle oefeningen correct opgelost."
+          other: "%{count} studenten hebben alle oefeningen correct opgelost."
+        teacher_progress_chart_info_tried:
+          one: "Eén student is met deze reeks gestart."
+          other: "%{count} studenten zijn met deze reeks gestart."
         progress_chart_info_correct:
           one: "Eén oefening correct opgelost."
           other: "%{count} oefeningen correct opgelost."

diff --git a/test/models/series_test.rb b/test/models/series_test.rb
@@ -722,4 +722,130 @@ class SeriesTest < ActiveSupport::TestCase
     create :correct_submission, exercise: series.exercises.second, user: user, course: series.course
     assert_equal 4, series.started_activity_count(user)
   end
+
+  test 'users_started should return the number of users that submitted to an exercise in the series or read a content page' do
+    series = create :series, exercise_count: 2, content_page_count: 2
+    unsubscribed_member = create :user
+    subscribed_member1 = create :user
+    series.course.course_memberships.create(user: subscribed_member1, status: :student)
+    subscribed_member2 = create :user
+    series.course.course_memberships.create(user: subscribed_member2, status: :student)
+    subscribed_member3 = create :user
+    series.course.course_memberships.create(user: subscribed_member3, status: :student)
+
+    assert_equal 0, series.users_started
+
+    create :activity_read_state, activity: series.content_pages.first, user: subscribed_member1, course: series.course
+    assert_equal 1, series.users_started
+
+    create :correct_submission, exercise: series.exercises.first, user: subscribed_member2, course: series.course
+    assert_equal 2, series.users_started
+
+    create :correct_submission, exercise: series.exercises.second, user: unsubscribed_member, course: series.course
+    assert_equal 2, series.users_started
+
+    create :activity_read_state, activity: series.content_pages.second, user: subscribed_member2, course: series.course
+    assert_equal 2, series.users_started
+
+    create :wrong_submission, exercise: series.exercises.second, user: subscribed_member3, course: series.course
+    assert_equal 3, series.users_started
+
+    create :correct_submission, exercise: series.exercises.second, user: unsubscribed_member, course: series.course
+    assert_equal 3, series.users_started
+  end
+
+  test 'users_completed should return the number of users that completed all activities in the series' do
+    series = create :series, exercise_count: 2, content_page_count: 2
+    unsubscribed_member = create :user
+    subscribed_member1 = create :user
+    series.course.course_memberships.create(user: subscribed_member1, status: :student)
+    subscribed_member2 = create :user
+    series.course.course_memberships.create(user: subscribed_member2, status: :student)
+    subscribed_member3 = create :user
+    series.course.course_memberships.create(user: subscribed_member3, status: :student)
+
+    assert_equal 0, series.users_completed
+
+    create :activity_read_state, activity: series.content_pages.first, user: subscribed_member1, course: series.course
+    create :correct_submission, exercise: series.exercises.first, user: subscribed_member2, course: series.course
+    create :correct_submission, exercise: series.exercises.second, user: unsubscribed_member, course: series.course
+    create :wrong_submission, exercise: series.exercises.first, user: subscribed_member3, course: series.course
+    assert_equal 0, series.users_completed # one exercise is not enough
+
+    create :correct_submission, exercise: series.exercises.second, user: subscribed_member2, course: series.course
+    create :activity_read_state, activity: series.content_pages.first, user: subscribed_member2, course: series.course
+    create :correct_submission, exercise: series.exercises.second, user: subscribed_member2, course: series.course
+    assert_equal 0, series.users_completed # subscribed_member2 has not completed the second content page
+
+    create :activity_read_state, activity: series.content_pages.second, user: subscribed_member2, course: series.course
+    assert_equal 1, series.users_completed # subscribed_member2 completed the series
+
+    create :wrong_submission, exercise: series.exercises.first, user: subscribed_member3, course: series.course
+    create :correct_submission, exercise: series.exercises.second, user: subscribed_member3, course: series.course
+    create :activity_read_state, activity: series.content_pages.first, user: subscribed_member3, course: series.course
+    create :activity_read_state, activity: series.content_pages.second, user: subscribed_member3, course: series.course
+    assert_equal 1, series.users_completed # subscribed_member3 has not completed the first exercise (wrong submission)
+
+    create :correct_submission, exercise: series.exercises.first, user: subscribed_member3, course: series.course
+    assert_equal 2, series.users_completed # subscribed_member3 completed the series
+
+    create :wrong_submission, exercise: series.exercises.first, user: subscribed_member3, course: series.course
+    assert_equal 1, series.users_completed # subscribed_member3 has no longer completed the series (wrong submission)
+
+    create :correct_submission, exercise: series.exercises.first, user: subscribed_member3
+    assert_equal 1, series.users_completed # submission is not in the same course
+
+    create :correct_submission, exercise: series.exercises.first, user: unsubscribed_member, course: series.course
+    create :correct_submission, exercise: series.exercises.second, user: unsubscribed_member, course: series.course
+    create :activity_read_state, activity: series.content_pages.first, user: unsubscribed_member, course: series.course
+    create :activity_read_state, activity: series.content_pages.second, user: unsubscribed_member, course: series.course
+    assert_equal 1, series.users_completed # unsubscribed_member has not subscribed to the course
+  end
+
+  test 'users_started should count submissions to activity in course, before activity was added to series' do
+    series = create :series, exercise_count: 0, content_page_count: 0
+    user = create :user
+    series.course.course_memberships.create(user: user, status: :student)
+    exercise = create :exercise
+
+    create :correct_submission, exercise: exercise, user: user, course: series.course
+    assert_equal 0, series.users_started
+
+    series.exercises << exercise
+    assert_equal 1, series.users_started
+    assert_equal 1, series.users_completed
+
+    content_page = create :content_page
+    create :series, course: series.course, content_pages: [content_page]
+    create :activity_read_state, activity: content_page, user: user, course: series.course
+    assert_equal 1, series.users_started
+    assert_equal 1, series.users_completed
+
+    series.content_pages << content_page
+    assert_equal 1, series.users_started
+    assert_equal 1, series.users_completed
+  end
+
+  test 'activity statuses should be updated after deadline change' do
+    series = create :series, exercise_count: 1, content_page_count: 1
+    user = create :user
+    series.course.course_memberships.create(user: user, status: :student)
+    exercise = series.exercises.first
+    content_page = series.content_pages.first
+
+    create :correct_submission, exercise: exercise, user: user, course: series.course
+    create :activity_read_state, activity: content_page, user: user, course: series.course
+
+    series.update!(deadline: 1.day.ago)
+    assert_equal true, ActivityStatus.find_by(activity: exercise, user: user, series: series).accepted
+    assert_equal true, ActivityStatus.find_by(activity: content_page, user: user, series: series).accepted
+    assert_equal false, ActivityStatus.find_by(activity: exercise, user: user, series: series).accepted_before_deadline
+    assert_equal false, ActivityStatus.find_by(activity: content_page, user: user, series: series).accepted_before_deadline
+
+    series.update!(deadline: 1.day.from_now)
+    assert_equal true, ActivityStatus.find_by(activity: exercise, user: user, series: series).accepted
+    assert_equal true, ActivityStatus.find_by(activity: content_page, user: user, series: series).accepted
+    assert_equal true, ActivityStatus.find_by(activity: exercise, user: user, series: series).accepted_before_deadline
+    assert_equal true, ActivityStatus.find_by(activity: content_page, user: user, series: series).accepted_before_deadline
+  end
 end