Fixed exam remaining time display offset

[rijuma]

Description

Ticket: COSMO-180 🔒

While taking an exam, the remaining time displayed in the exam timer is inconsistent. It gets an increasing offset when navigating around in a course in a way that requires a browser refresh. It appears that this does not occur when the browser does not refresh (e.g. navigating between units in the same subsection via the navigation bar). It’s usually up to a 40 second difference in either direction.

Cause

The cause for this offset was caused by the fact that we weren't using a fixed time reference for the remaining time. The endpoint returns the remaining time in seconds which was saved in the redux store, and then when the TimerProvider was rendered it was calculating the remaining time based on the current time. The first render would be accurate but after that each re-render of the component would take the same value to start the timer. This caused an offset which eventually would be corrected by a poll that happens every minute, which mitigates but does not solve the problem.

Proposed solution

In order to fix this, my approach is to calculate a timerEnds date at the moment we store this information in Redux, and then use this timer to calculate the seconds to show to the user. Since the date is fixed in time, no offset is created no matter the reloads.

Extra changes

There are some minor issues that I detected wile working on this. For example, sometimes the frontend wanted to poll periodically to an url that was returned by the endpoint exam_started_poll_url, which was indicated by a comment in the code poll url may be null if this is an LTI exam. In my experience, these cases were showing an obscure error to the user.

I've used this opportunity to reshape the error in a more descriptive way:

We are now avoiding the poll when the url is not provided to avoid this error overall, but I thought it was a nice thing to keep just in case.

Update (2024-02-27)

• When updating the tests I've added act() to wrap the await expect() to wait for update changes, since there are state changes involved and jest expects us to use act() when this happens.
• Added some tweaks that might raise an eyebrow, especially on the timeless useTimeout() to wait for the children render to subscribe to the emitter. Please check it when reviewing.

Update (2024-02-29)

• Added 100% coverage to TimeProvider. It didn't had any coverage before.
• Made some extra tweaks to the code while testing. I feel pretty confident that it works as it should.
• Note: There's a weird choice on how the ping was working, if the ping_interval is set, it runs first at half of the time, and then each full interval. I left it as it is, but I wanted to bring attention to it.
• Added a test:watch to package.json's scripts that's handy when working on new tests. The coverage output gets too much in the way.

[rijuma]

I've moved this to be handled in the interval block, since I find it easier to understand.

[rijuma]

The control variable was moved to the useEffect block, since the component refresh would overwrite this value rendering it useless.

[zacharis278]

Great, the approach looks pretty good. I'd take a look at @ilee2u's PR about the error cropping up. #139. I think both of these problems are slightly related since it wasn't always hitting the backend to sync up like it should have

[codecov]

Codecov Report

Attention: Patch coverage is 98.14815% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 93.86%. Comparing base (7cf0be5) to head (616158e).

Files	Patch %	Lines
src/data/thunks.js	50.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #140      +/-   ##
==========================================
+ Coverage   93.25%   93.86%   +0.61%     
==========================================
  Files          68       68              
  Lines        1052     1076      +24     
  Branches      291      295       +4     
==========================================
+ Hits          981     1010      +29     
+ Misses         66       61       -5     
  Partials        5        5              

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

[zacharis278]

this looks good. REALLY appreciate getting test coverage on the TimerProvider component, thank you!

Few small comments but otherwise approved 👍