Display Statistics View per Student for their Submission Time by bivanalhar · Pull Request #7222 · Coursemology/coursemology2

bivanalhar · 2024-03-26T03:02:48Z

Feature

We implemented Submission Time Due Table, which basically shows the submission status for each published, non-submitted assessments for any particular student. The objective is to give tutors or teachers the overview of which assessments have not yet been done by the students, and how far it is from the due date.

This feature is requested as part of the effort in knowing which student needs some extra help in finishing up each assessment, and also to give tutors time to warn the students to finish up their assignments, preferably before the due date.

Overview

Since we want to focus on each student, the design, as well as the entry point, of this page is quite different from the other Course Statistics Page, as follows:

Firstly, you need to go to the Course Statistics Students page
Then, for the student that you might be interested in knowing their not-yet-submitted assessments, you can just click on their name, then the new tab will open, showing all those assessments

- API merely sketch from FE (will complete implementation in BE) - mock page for assessment statistics - routes for assessments statistics fetching API - at this point, only mock data is provided to FE from BE

- gather all info in BE, sending to FE - code cleaning in BE

- using sortable and searchable table

- also fix the styling in concern files - default of avg and stdev to 0 if assessment_id not found in hash

- include submitted also in counting attempting submissions - use SQL to extract maximum grade

- already moved to index.tsx - add Assessment Tab in index.tsx - delete courseStatistics.js as already migrated to typescript

- it should match with the URL provided

- API merely sketch from FE (will complete implementation in BE) - mock page for assessment statistics - routes for assessments statistics fetching API - at this point, only mock data is provided to FE from BE

- gather all info in BE, sending to FE - code cleaning in BE

- extend users API to include extract only students basic info - prepare the Page for Submission Time (autocomplete field and space for table)

- needed for submission time table, to indicate the following - if assessment has started respective to student's personal time

- depending on which student is chosen

- fix rubocop issue

- course_user_id constrained only within current course - add test for submission time

- to display properly and show user if the timeline is personalised or not

- remove tab for submission time, entry through students stats table - table design change to provide due date and due in - include personalised indicator and real deadline - change API routing for getting submission_time

cysjonathan · 2024-04-18T10:07:43Z

    format("%2d #{t('time.day')} %02d:%02d:%02d", days, hours, minutes, seconds)
  end
+
+  def time_until_due(submitted_at, end_at)


This function behaves as time_delta between datetimestamp1 and datetimestamp2, it shouldn't be specific to time_until_due.
Also I don't think it makes sense for end_at - expected submission_time to represent "time until due".

Can we change this to be a time_delta instead? Or consider not having a function for this?

I think time_delta is a very general phrase, even though what this function represents is actually how far the submitted_at is from the end_at.

The usage of time_until_due might be communicating the ideas better to the developer.

The point was that time_until_due(submitted_at, end_at) is behaving as a general function, time delta represents how far datetimestamp1 (submitted_at) is from datetimestamp2 (end_at).

time_until_due function signature sounds like time_until_due(assessment_id, student_id) which reports the time until the assessment is due for the given student (default time if no student is provided, or no personal times).

@ekowidianto your thoughts?

changed into calculate_time_interval and also the arguments naming is revised to from and optional_to

cysjonathan · 2024-04-18T10:08:23Z

+    json.workflowState 'unstarted'
+  end
+
+  json.dueIn time_until_due(Time.now, end_at)


would Time.now - end_at be sufficient?

might be sufficient but I'm afraid the code's not communicative enough (right now it's just using Time.now since we are not considering the submitted/graded/published submissions).

But for the future, if we want to consider those kinds of submissions as well, the function time_until_due might be more general and can be used for that case as well.

cysjonathan · 2024-04-18T10:20:44Z

+json.name @all_students.first.name
+
+json.assessments @assessments do |assessment|
+  student_id = @all_students.first.id


why are we computing student_id inside the do-loop?

oh ya sorry about this. this can be put also outside the do-loop, and might be saving the execution time a bit.

Even though the effect might not be significant, but I moved this out from the loop

cysjonathan · 2024-04-18T10:22:19Z

  end

+  def submission_due
+    @all_students = [CourseUser.where(id: params[:student_id]).first]


Why is @all_students only capturing one student?

this is because this submission_due API only accept one student_id from the frontend (the way this API is called is firstly to specify which student we would like to know more).

And the reason I'm using @all_students here is because my intention is to reuse the function all_submissions_info

cysjonathan · 2024-04-18T10:22:45Z

    fetch_all_assessment_related_statistics_hash
  end

+  def submission_due


submissions_due instead of submission_due? Since we expect multiple submissions.

I'm thinking that submission_due is also correct since we only expect 1 submission per assessment.
Nevertheless, I can change this into what you suggested as well

cysjonathan · 2024-04-18T10:30:35Z

+  Safe: 'safe' as const,
+  AlmostLate: 'almostLate' as const,
+  Overdue: 'overdue' as const,


NotDue, AlmostDue, OverDue.
^ just a suggestion. Keeping the "xxxDue" convention (two words) makes it easier for developers to remember.

You got the point there, alright I'll change it.

cysjonathan · 2024-04-19T02:58:54Z

+
+  let dueStates = '';
+
+  if (days >= 3 && rawTime >= 0) {


Go into constants file, perhaps AlmostDueThreshold or something similar, instead of here.

got it. I've changed this

cysjonathan · 2024-04-19T03:19:08Z

      personal_times AS (
-        SELECT cu.id AS course_user_id, pt.end_at, pt.assessment_id
+        SELECT cu.id AS course_user_id, pt.start_at, pt.end_at, pt.fixed, pt.assessment_id
        FROM (
          SELECT course_users.id
          FROM course_users
          WHERE course_users.course_id = #{course_id}
        ) cu
        LEFT JOIN (
-          SELECT course_user_id, end_at, assessment_id
+          SELECT course_user_id, start_at, end_at, fixed, assessment_id
          FROM course_user_personal_end_at
        ) pt
        ON cu.id = pt.course_user_id
      ),


Why need nested subquery?
Can we do:

personal_times AS ( SELECT cu.id AS course_user_id, pt.start_at, pt.end_at, pt.fixed, pt.assessment_id FROM course_users cu LEFT JOIN course_user_personal_end_at pt ON cu.id = pt.course_user_id WHERE cu.course_id = #{course_id} )```

Same for the other queries

when making this nested subquery, my consideration is to make the code neater to read (having the code just like how u wrote it looks very condensed and not really that nice to read).

However, I think this is just about preference and I have adjusted the code to be like what you suggested above

cysjonathan · 2024-04-19T03:20:57Z

+      _, end_at, = @personal_timeline_hash[[@assessment.id, submitter_course_user.id]] ||
+                   @reference_times_hash[@assessment.id]


Does this account for Multiple Reference Timelines (as in the timeline designer)?

yes, this functionality considers the personalised timeline to be the priority, then only using reference timeline if there's no personalised one

cysjonathan · 2024-04-19T03:37:27Z

+          expected_end_at = published_assessment.end_at.change(usec: published_assessment.end_at.usec / 1000 * 1000)
+          parsed_end_at = DateTime.parse(json_result['assessments'][0]['endAt']).
+                          change(usec: DateTime.parse(json_result['assessments'][0]['endAt']).
+                                       usec / 1000 * 1000)
+          parsed_reference_end_at = DateTime.parse(json_result['assessments'][0]['referenceEndAt']).
+                                    change(usec: DateTime.parse(json_result['assessments'][0]['referenceEndAt']).
+                                                 usec / 1000 * 1000)
+
+          expect(json_result['assessments'][0]['workflowState']).to eq('attempting')
+          expect(parsed_end_at).to eq(expected_end_at)
+          expect(parsed_reference_end_at).to eq(expected_end_at)


isn't there a .to be_within(time_delta) method that we can use instead of doing truncation like this?

you're right. I have done some more inspection and indeed, previously I was using this be_within function but I was doing it wrongly and thus it's unsuccessful

now that I have done it properly, it gets right now

ekowidianto · 2024-04-19T03:53:01Z

 # This is named aggregate controller as naming this as course controller leads to name conflict issues
 class Course::Statistics::AggregateController < Course::Statistics::Controller
  before_action :preload_levels, only: [:all_students, :course_performance]
+  before_action :fetch_published_assessments, only: [:all_assessments, :submission_due]


Suggested change

before_action :fetch_published_assessments, only: [:all_assessments, :submission_due]

before_action :preload_assessments, only: [:all_assessments, :submission_due]

be consistent in the naming. see line 4 above.

got it.. though I will change it into preload_published_assessments since we actually only care about the published one, and ignoring the unpublished.

- simplify SQL script not to use redundant nesting code - submission_due -> submissions_due - fetch -> preload to make it consistent with other similar functions - naming of dueInStates to be more consistent (notDue, almostDue, overDue)

- student name in student stats link to user page - add one column for clickable icon, link to overdue assessments

bivanalhar force-pushed the bivan/course-stats-submission-time branch 2 times, most recently from 249dc8e to 7d4ad4c Compare March 26, 2024 05:41

bivanalhar requested a review from ekowidianto March 27, 2024 05:22

bivanalhar added 9 commits April 17, 2024 18:13

feat(assessment_statistics): provide page and API

b6bb2b3

- API merely sketch from FE (will complete implementation in BE) - mock page for assessment statistics - routes for assessments statistics fetching API - at this point, only mock data is provided to FE from BE

feat(assessment_stats): get all statistics info

393336a

- gather all info in BE, sending to FE - code cleaning in BE

feat(assessment_statistics): display table

d0c6508

- using sortable and searchable table

fix(style): numLate before numAttempting

e1d9956

- also fix the styling in concern files - default of avg and stdev to 0 if assessment_id not found in hash

refactor(stats): change in attempting

1607d10

- include submitted also in counting attempting submissions - use SQL to extract maximum grade

test(assessment): test for course stats assessment

ed4224f

feat(api): add API for course stats assessment

dd15b43

feat(api): add API for course stats assessment

723ef62

fix(stats): remove statisticsTabs since unused

b7b2109

- already moved to index.tsx - add Assessment Tab in index.tsx - delete courseStatistics.js as already migrated to typescript

bivanalhar force-pushed the bivan/course-stats-assessment branch from 34d2faf to 1c28e44 Compare April 17, 2024 10:14

bivanalhar force-pushed the bivan/course-stats-submission-time branch from c813d8c to 6233460 Compare April 17, 2024 10:23

bivanalhar marked this pull request as draft April 17, 2024 10:24

bivanalhar removed the request for review from ekowidianto April 17, 2024 10:24

fix(tabs): default chosen tab should not be always 0

19f54cd

- it should match with the URL provided

bivanalhar force-pushed the bivan/course-stats-assessment branch from 1c28e44 to 19f54cd Compare April 18, 2024 09:14

bivanalhar added 11 commits April 18, 2024 17:28

feat(assessment_statistics): provide page and API

30cd6df

- API merely sketch from FE (will complete implementation in BE) - mock page for assessment statistics - routes for assessments statistics fetching API - at this point, only mock data is provided to FE from BE

feat(assessment_stats): get all statistics info

808ec4b

- gather all info in BE, sending to FE - code cleaning in BE

feat(stats): add tab for submission time info

efab987

- extend users API to include extract only students basic info - prepare the Page for Submission Time (autocomplete field and space for table)

refactor(reference-time): include start_at

baae0f7

- needed for submission time table, to indicate the following - if assessment has started respective to student's personal time

feat(API): create API for fetching submission time

dd636b8

- depending on which student is chosen

feat(submission_time): get submission time info in BE

ce1bbbc

feat(translation): add Unreleased into status translations

6aac12b

feat(submission_time): add Submission Time Table

a931cfb

- fix rubocop issue

fix(submission): fix and add test

e55d749

- course_user_id constrained only within current course - add test for submission time

feat(timeline): pass isFixed and personalised time to FE

ef92f92

- to display properly and show user if the timeline is personalised or not

feat(submission_time): change route and page

598cc96

- remove tab for submission time, entry through students stats table - table design change to provide due date and due in - include personalised indicator and real deadline - change API routing for getting submission_time

bivanalhar force-pushed the bivan/course-stats-submission-time branch from 6233460 to 0f1c5c7 Compare April 18, 2024 09:29

bivanalhar force-pushed the bivan/course-stats-submission-time branch 2 times, most recently from a53a27b to da9d584 Compare April 19, 2024 01:50

bivanalhar marked this pull request as ready for review April 19, 2024 01:54

bivanalhar requested review from cysjonathan and ekowidianto April 19, 2024 01:54

bivanalhar force-pushed the bivan/course-stats-submission-time branch 2 times, most recently from 3b1494c to 22d409d Compare April 19, 2024 03:00

cysjonathan reviewed Apr 19, 2024

View reviewed changes

fix(naming): submission_time -> submission_due

22a3868

bivanalhar force-pushed the bivan/course-stats-submission-time branch from 22d409d to 22a3868 Compare April 19, 2024 03:23

cysjonathan reviewed Apr 19, 2024

View reviewed changes

ekowidianto reviewed Apr 19, 2024

View reviewed changes

chore(naming): some minor changes in name

a7d2630

- simplify SQL script not to use redundant nesting code - submission_due -> submissions_due - fetch -> preload to make it consistent with other similar functions - naming of dueInStates to be more consistent (notDue, almostDue, overDue)

bivanalhar requested review from cysjonathan and ekowidianto May 7, 2024 06:04

refactor(submission-time): change UI

ef1aac7

- student name in student stats link to user page - add one column for clickable icon, link to overdue assessments

bivanalhar force-pushed the bivan/course-stats-assessment branch 8 times, most recently from a95ee0c to 4226179 Compare August 7, 2024 04:06

bivanalhar force-pushed the bivan/course-stats-assessment branch 3 times, most recently from 13d478b to aa23e0c Compare September 9, 2024 22:58

cysjonathan force-pushed the bivan/course-stats-assessment branch from aa23e0c to 62e2ebf Compare September 12, 2024 04:52

Base automatically changed from bivan/course-stats-assessment to master September 12, 2024 06:08

cysjonathan marked this pull request as draft October 28, 2024 09:01

		_, end_at, = @personal_timeline_hash[[@assessment.id, submitter_course_user.id]] \|\|
		@reference_times_hash[@assessment.id]

	before_action :fetch_published_assessments, only: [:all_assessments, :submission_due]
	before_action :preload_assessments, only: [:all_assessments, :submission_due]

Conversation

bivanalhar commented Mar 26, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Feature

Overview

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

bivanalhar commented Mar 26, 2024 •

edited

Loading