Skip to content
This repository has been archived by the owner on Jun 16, 2018. It is now read-only.

Common ranking #295

Merged
merged 7 commits into from Nov 4, 2017
Merged

Common ranking #295

merged 7 commits into from Nov 4, 2017

Conversation

poelstra
Copy link
Collaborator

Part of #245: Move Ranking computation to common code (used by ng-scores or a new ng-ranking service)

All tests pass for each commit.

The first few commits are only refactors, no functional changes.
The last 2 fix two small bugs.

To test:

  • For the first commits: check that the rankings still work in the frontend
  • For the negative round bug: edit a score to have a round of 0 or lower, verify that it no longer ends up in rankings
  • For the crash in CSV export bug: edit a score to have a non-existing team number, verify that there are no errors in JS Console, and Export button on Ranking screen works (team info will be empty for that score, though)

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 3c26de4 on common_ranking into ** on master**.

@kmeesters
Copy link
Member

Test (npm test) and through the frond-end. (win 10, node 8.6.0). Works okay.
Questions though is what to do with scores with incorrect team-numbers, should they disappear from ranking like when the round-number is correct? I.e. prohibit publishing of scores if there is an error? Or should we not force it? However will make an issue for this, for now the current functionality is fine.

@kmeesters kmeesters merged commit 50c5621 into master Nov 4, 2017
@kmeesters kmeesters deleted the common_ranking branch November 4, 2017 20:58
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants