Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Compute time spent per question #137

Merged
merged 8 commits into from
Nov 21, 2023
Merged

Compute time spent per question #137

merged 8 commits into from
Nov 21, 2023

Conversation

suryabulusu
Copy link
Collaborator

@suryabulusu suryabulusu commented Oct 26, 2023

Fixes #123

Summary

  • Computes time spent on each question for homework and assessment types
  • For OMR assessment, we ignore this computation.

We maintain an array timeSpentPerQuestion[] that has a length of numQuestions.

For each question in homework,

  • timer starts when question is opened (via Start/Resume/Continue buttons)
  • timer stops when answer is submitted (Submit button) or when back button is clicked.
  • time_spent is sent to backend with "Submit" button is clicked.

For each question in assessment,

  • timer starts when question is opened (via Start/Resume/Back/Next/Palette buttons)
  • timer stops when navigated to another question
  • time_spent is sent to backend when Save & Next button is clicked.

In addition, we also send an update on time_spent on each question to backend every few seconds, along with dummy-event. This helps keep track of the latest values of timeSpentPerQuestion (in case submit or save buttons are not clicked).

Related ADR: https://www.notion.so/avantifellows/ADR-Time-Spent-Per-Question-211b3f4088894f8b96b471a787c2f073

Test Plan

  • Test Responsiveness
    • Laptop (1200px)
    • Tablet (760px)
    • Phone (320px)
  • Cross-Browser Testing
    • Chrome
    • Firefox
  • Local Language Support
  • Hygiene and Housekeeping
    • Self-review
    • Comments have been added appropriately
    • Check for bundle size here if adding a package
    • Added relevant details like Labels/Projects/Milestones etc.
    • If adding or removing any environment variable, update docs/ENV.md, .env.example and the Github workflows.
  • Testing
    • Wrote tests
    • Tested locally
    • Tested on staging
    • Tested on an actual physical phone
    • Tested on production
  • Lighthouse Checks
    • Images have alt attributes
    • Any <img> tags have width and height specified
    • Any target="_blank" links have rel="noopener"
    • Only SVGs are used as images. If PNGs are used, their size has been optimised.
    • Any SVG buttons without text have their aria-label attributes set

@codecov
Copy link

codecov bot commented Oct 26, 2023

Codecov Report

Attention: 10 lines in your changes are missing coverage. Please review.

Comparison is base (b3ef2b4) 73.87% compared to head (0398339) 78.72%.

Files Patch % Lines
src/views/Player.vue 81.81% 10 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #137      +/-   ##
==========================================
+ Coverage   73.87%   78.72%   +4.84%     
==========================================
  Files          59       17      -42     
  Lines        7626     1349    -6277     
  Branches      801      394     -407     
==========================================
- Hits         5634     1062    -4572     
+ Misses       1970      287    -1683     
+ Partials       22        0      -22     
Flag Coverage Δ
chrome 78.72% <81.81%> (+4.84%) ⬆️
ui 78.72% <81.81%> (+1.12%) ⬆️
unittest ?

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@suryabulusu suryabulusu temporarily deployed to Staging October 26, 2023 17:37 — with GitHub Actions Inactive
src/views/Player.vue Outdated Show resolved Hide resolved
Copy link

@pritamps pritamps left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just small change, then looks good!

Copy link

@pritamps pritamps left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me as far as I can tell. But I've lost context here :(

@suryabulusu suryabulusu merged commit ba75319 into main Nov 21, 2023
7 of 9 checks passed
@suryabulusu suryabulusu deleted the time_spent_per_ques branch November 21, 2023 08:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Record time spent on each question
2 participants