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

Update points if there is server-client discrepancy #4970

Merged

Conversation

MCGallaspy
Copy link
Contributor

Summary

Fixes error in points display. For exercise log sync events, only update the points display if there is a discrepancy between the points reported by the server and the points known to the client.

TODO

If not all TODOs are marked, this PR is considered WIP (work in progress)

  • Have tests been written for the new code? If you're fixing a bug, write a regression test (or have a really good reason for not writing one... and I mean really good!)

Here's my really good reason. Testing this using selenium would take the following steps:

  1. Create new student account and set the points to some appropriate initial value.
  2. Navigate to an exercise.
  3. Interact with the exercise to gain points.
  4. Make assertion about observed points in user menu.

This is challenging because we don't provide a testing api for interacting with content nor a reliable way to determine the "state" of the page in order to avoid race conditions (e.g. polling the #points element too soon or not waiting long enough after a page is loaded). I also wrote a similar test that was removed. I think I can write a better, less error-prone test now in about a day. I'm on the fence as to whether that's a good use of time right now. Dear reviewer, what do you think?

Issues addressed

Fixes #4643

@rtibbles
Copy link
Member

rtibbles commented Mar 8, 2016

I think it is probably not a great use of our time. Given that we have already accepted that we need manual testing, I would add this to the list of things that an exercise tester should check for.

rtibbles added a commit that referenced this pull request Mar 9, 2016
Update points if there is server-client discrepancy
@rtibbles rtibbles merged commit 985e3f3 into learningequality:0.16.x Mar 9, 2016
@rtibbles rtibbles removed the has PR label Mar 9, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants