scores: Enable automatic refreshing (every 30s for now) of scores.json. #281
Conversation
spec/services/ng-scoresSpec.js
Outdated
| $scores.enableAutoRefresh(); | ||
| $scores.beginupdate(); | ||
| $interval.flush($scores._autoRefreshTimeout + 1); | ||
| expect(fsMock.read).not.toHaveBeenCalled(); |
There was a problem hiding this comment.
missing a test that it should continue refreshing when no longer busy
src/js/services/ng-scores.js
Outdated
| this._autoRefreshInterval = $interval(function() { | ||
| // Reload scores from server (if we're not already | ||
| // in the middle of something). | ||
| if (self._updating <= 0) { |
There was a problem hiding this comment.
Better extract a function isUpdating() that checks the _updating counter, rather than comparing the counter against 0 on several places
|
To reduce the code complexity a bit, I'd suggest to extract the 3 methods to something generic and external to the module. Basically is is a fancy $interval service that keeps track of subscriber count. The only parameters needed by this service is what to do every interval: And the interval itself. All the rest is internal to the service |
- Functionality works (in browser) - Tests currently fail because ng-scores can't find Poller (only in tests)
|
@rikkertkoppes Addressed both of your remarks. However, the tests for ng-scores now fail. Apparently, because ng-scores can't find the Poller factory, but this only happens in the tests. I've been pulling my hair out for 2 hours on this now, and can't figure out why. I would really appreciate if you can take a look at why this may be happening, given that you initially set that stuff up, and are more knowledgeable on the angular side anyway... |
|
BTW: the conflicts are trivial to resolve: they are only marked as conflict because the lines happened to be adjacent. So keeping changes from both sides will do the trick. |
|
Tests issue should be fixed by #293, will merge master into this (or rebase) after that PR is merged. |
Conflicts: spec/mocks/scoresMock.js src/js/views/scores.js
|
Appears that another dependency was missing ('services' should depend on 'factories'), which fixes the tests (even without #293). Strange that it worked in the frontend without that dependency, though... Merged master in to resolve the merge conflicts (basically accepting the changes from both sides). |
Conflicts: src/js/services/ng-scores.js
poelstra
force-pushed
the
periodic_scores_refresh
branch
from
13ac3de
to
eb4582b
Compare
Another part of #245: automatically refresh the scores and ranking screens.
Note: this PR only does a periodic refresh (hardcoded to 30s for now).
It does not yet refresh on e.g. an MHub message, which can be done separately.
It also doesn't yet remove the Refresh button: I think it may occassionally still be useful to manually press it if needed, but let's discuss that in a separate issue.
Advice to merge this PR after #278, to prevent people being annoyed when a refresh happens when they're editing a score.
Also fixes #115.