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

Added the ability to control how score gets exported #262

Open
ThinkRedstone wants to merge 12 commits into
base: master
Choose a base branch
from
Open

Added the ability to control how score gets exported #262

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
f4b8fe2
Changed sorting icons to a material-icons versions in ranking
ThinkRedstone Aug 20, 2017
fe8f09a
added sorting icons to scores.html
ThinkRedstone Aug 20, 2017
2800b81
changed test for ranking to use the new material-icons
ThinkRedstone Aug 20, 2017
588bd38
changed score.index to $index in scores.html
ThinkRedstone Aug 20, 2017
9efa240
Moved css properties of sortable fields to elements.css under sortabl…
ThinkRedstone Aug 20, 2017
b5e75f5
Removed broken export dialog
ThinkRedstone Aug 21, 2017
b779a51
Added settings for formatting exported scores
ThinkRedstone Aug 21, 2017
5a0d236
Changed export function to abide user settings
ThinkRedstone Aug 22, 2017
fb41f47
Merge branch 'sorting-icons' into export-score
ThinkRedstone Aug 22, 2017
4b92580
made array encoding notepad-friendly
ThinkRedstone Aug 22, 2017
e1cae74
Replaces undefined or null fields with an empty string in exported files
ThinkRedstone Aug 22, 2017
a3c4b78
Changed test in accordance with the new export file format and expand…
ThinkRedstone Aug 22, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
72 changes: 43 additions & 29 deletions spec/views/rankingSpec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,32 +10,38 @@ describe('ranking', function() {
number: '123',
name: 'foo'
};
var fsMock, stagesMock, scoresMock, handshakeMock, messageMock;
var fsMock, stagesMock, scoresMock, handshakeMock, messageMock, settingsMock;

beforeEach(function() {
angular.mock.module(module.name);
angular.mock.inject(function($controller, $rootScope,$q) {
$scope = $rootScope.$new();
scoresMock = createScoresMock($q);
handshakeMock = createHandshakeMock($q);
stagesMock = createStagesMock();
stagesMock = createStagesMock($q);
Copy link
Collaborator

@poelstra poelstra Aug 28, 2017

Choose a reason for hiding this comment

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

It's not ideal to have to pass the promise implementation to the mock.
Just use Promise inside of it, and/or the 'normal' q package.

Copy link
Member

@rikkertkoppes rikkertkoppes Aug 28, 2017

Choose a reason for hiding this comment

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

I believe this is legacy from our side. Not sure how to fix that as the stages services used $q to resolve promises in an "angular aware" fashion.

The other option would be to not use $q at all in the service, but wrap its use in $q.when() everywhere, but that seems even less ideal

messageMock = createMessageMock();
var settings = {};
settings.lineStartString = "\"";
settings.lineEndString = "\"";
settings.separatorString = "\",\"";
settingsMock = createSettingsMock($q, settings);
controller = $controller('rankingCtrl', {
'$scope': $scope,
'$scores': scoresMock,
'$stages': stagesMock,
'$handshake': handshakeMock,
'$message': messageMock
'$message': messageMock,
'$settings': settingsMock,
});
});
$scope.$digest();//resolves all init promises, etc.
});

describe('initialization', function() {
it('should initialize', function() {
expect($scope.sort).toEqual('rank');
expect($scope.rev).toEqual(false);
expect($scope.csvdata).toEqual({});
expect($scope.csvname).toEqual({});
expect($scope.exportFiles).toEqual({});
});
});

Expand Down Expand Up @@ -79,23 +85,23 @@ describe('ranking', function() {
sort: 'foo'
};
expect($scope.sortIcon(stage)).toEqual('');
expect($scope.sortIcon(stage,'foo')).toEqual('icon-sort-up');
expect($scope.sortIcon(stage,'foo')).toEqual('arrow_drop_up');
});
it('should give the up icon when col is sorted in reverse',function() {
var stage = {
sort: 'foo',
rev: true
};
expect($scope.sortIcon(stage)).toEqual('');
expect($scope.sortIcon(stage,'foo')).toEqual('icon-sort-down');
expect($scope.sortIcon(stage,'foo')).toEqual('arrow_drop_down');
});

//default sort order stuff, needs a bit of refactoring
it('should report a default sorting for any stage',function() {
var stage = {};
expect($scope.sortIcon(stage,'rank')).toEqual('icon-sort-up');
expect($scope.sortIcon(stage,'rank')).toEqual('arrow_drop_up');
$scope.rev = true;
expect($scope.sortIcon(stage,'rank')).toEqual('icon-sort-down');
expect($scope.sortIcon(stage,'rank')).toEqual('arrow_drop_down');
});
});

Expand Down Expand Up @@ -134,47 +140,55 @@ describe('ranking', function() {
});
});

describe('rebuildCSV',function() {
it('should generate CSV data and filenames',function() {
expect($scope.csvname).toEqual({});
expect($scope.csvdata).toEqual({});
$scope.rebuildCSV({
describe('buildExportFiles',function() {
it('should generate export files',function() {
expect($scope.exportFiles).toEqual({});

$scope.scoreboard = {
'qualifying': [
{ rank: 1, team: { name: "foo", number: 123 }, highest: 10, scores: [0, 10, 5] },
{ rank: 1, team: { name: "\"bar\"", number: 456 }, highest: 10, scores: [10, 0, 5] }
]
});
expect($scope.csvname["qualifying"]).toEqual("ranking_qualifying.csv");
expect($scope.csvdata["qualifying"]).toEqual("data:text/csv;charset=utf-8," + encodeURIComponent([
'"Rank","Team Number","Team Name","Highest","Round 1","Round 2","Round 3"',
};
$scope.buildExportFiles();
expect($scope.exportFiles["qualifying"]).toEqual("data:text/csv;charset=utf-8," + encodeURIComponent([
'"1","123","foo","10","0","10","5"',
'"1","456","""bar""","10","10","0","5"',
].join("\r\n")));
'"1","456",""bar"","10","10","0","5"', //new format doesn't replace every quotation mark with two
].join("\r\n").concat("\r\n")));//new format ends in a newline
});
it('should not skip empty values, but include as empty string',function() {
$scope.rebuildCSV({
$scope.scoreboard = {
'qualifying': [
{ team: { name: "foo", number: 123 }, highest: 10, scores: [0, 10, 5] },
{ team: { name: "\"bar\"", number: 456 }, highest: 10, scores: [10, 0, 5] }
]
});
expect($scope.csvdata["qualifying"]).toEqual("data:text/csv;charset=utf-8," + encodeURIComponent([
'"Rank","Team Number","Team Name","Highest","Round 1","Round 2","Round 3"',
};
$scope.$digest();
expect($scope.exportFiles["qualifying"]).toEqual("data:text/csv;charset=utf-8," + encodeURIComponent([
'"","123","foo","10","0","10","5"',
'"","456","""bar""","10","10","0","5"',
].join("\r\n")));
'"","456",""bar"","10","10","0","5"', //new format doesn't replace every quotation mark with two
].join("\r\n").concat("\r\n")));//new format ends in a newline
});
});

describe('scoreboard watcher',function() {
it('should rebuild the csv when the scoreboard changes',function() {
$scope.rebuildCSV = jasmine.createSpy('rebuildCSV');
it('should rebuild the export files when the scoreboard changes',function() {
$scope.buildExportFiles = jasmine.createSpy('buildExportFiles');
$scope.scoreboard = 'foo';
$scope.$digest();
expect($scope.rebuildCSV).toHaveBeenCalledWith(scoresMock.scoreboard);
expect($scope.buildExportFiles).toHaveBeenCalled();
});
});

describe('settings watcher', function () {
it('should rebuild the export files when the settings change', function () {
$scope.buildExportFiles = jasmine.createSpy('buildExportFiles');
$scope.settings.bla = "fo";
$scope.$digest();
expect($scope.buildExportFiles).toHaveBeenCalled();
})
});

describe('getRoundLabel',function() {
it('should create a label for rounds',function() {
expect($scope.getRoundLabel(4)).toEqual('Round 4');
Expand Down
8 changes: 8 additions & 0 deletions src/css/elements.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -327,3 +327,11 @@ button .material-icons {
display: none !important;
}
}

.sortable span{
float: left;
}

.sortable i{
float: right;
}
98 changes: 47 additions & 51 deletions src/js/views/ranking.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,13 @@ define('views/ranking',[
'services/ng-scores',
'services/ng-handshake',
'services/ng-message',
'controllers/ExportRankingDialogController',
'services/ng-settings',
'angular'
],function(log) {
var moduleName = 'ranking';
return angular.module(moduleName,['ExportRankingDialog']).controller(moduleName+'Ctrl', [
'$scope', '$scores', '$stages','$handshake','$message',
function($scope, $scores, $stages, $handshake, $message) {
return angular.module(moduleName,[]).controller(moduleName+'Ctrl', [
'$scope', '$scores', '$stages','$handshake','$message', '$settings',
function($scope, $scores, $stages, $handshake, $message, $settings) {
log('init ranking ctrl');

// temporary default sort values
Expand Down Expand Up @@ -70,15 +70,15 @@ define('views/ranking',[
var icon = '';
if (stage.sort == col) {
if (stage.rev){
icon = 'icon-sort-down';
icon = 'arrow_drop_down';
} else {
icon = 'icon-sort-up';
icon = 'arrow_drop_up';
}
} else if (stage.sort === undefined && col == $scope.sort) {
if (stage.rev === undefined && $scope.rev) {
icon = 'icon-sort-down';
icon = 'arrow_drop_down';
} else {
icon = 'icon-sort-up';
icon = 'arrow_drop_up';
}
} else {
icon = ''; // no icon if column is not sorted
Expand All @@ -102,64 +102,60 @@ define('views/ranking',[
return new Array($scope.maxRounds() - stage.$rounds.length);
};

// Data for CSV export links, indexed by stage ID
$scope.csvdata = {}; // CSV data itself
$scope.csvname = {}; // Filenames suggested to user

// Convert a 2D matrix to a CSV string.
// All cells are converted to strings and fully quoted,
// except null or undefined cells, which are passed as empty
// values (without quotes).
function toCSV(rows) {
return rows.map(function(row) {
return row.map(function(col) {
// Escape quotes, and wrap in quotes
if (col === undefined || col === null) {
col = "";
}
return '"' + String(col).replace(/"/gi, '""') + '"';
}).join(",");
}).join("\r\n"); // Use Windows line-endings, to make it Notepad-friendly
}
/**
* encodes a two dimensional array as a string according to the settings
* specified by the user as reported by the ng-settings.$settings service
*
* @param array the array to be encoded
* @returns {string} the encoded string form of the array
*/
$scope.encodeArray = function (array) {
var string = "";
var settings = $settings.settings;
Copy link
Collaborator

@poelstra poelstra Aug 28, 2017

Choose a reason for hiding this comment

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

encodeArray now 'depends' on $settings, making it harder to refactor out of here and re-use elsewhere, and harder to unit-test.
Better to explicitly pass the settings in.

array.forEach(function (row) {
row = row.map((elem) => elem || elem === 0 ? String(elem) : "");
string = string.concat(settings.lineStartString ? String(settings.lineStartString) : "");
string = string.concat(row.join(settings.separatorString ? String(settings.separatorString) : ""));
string = string.concat((settings.lineEndString ? String(settings.lineEndString) : "") + "\r\n");
});
return string;
};

$scope.exportFiles = {};

/**
* Rebuild CSV data (contents and filenames) of given scoreboard.
* @param scoreboard Per-stage ranking as present in e.g. $scores.scoreboard.
* Builds the .csv file for exporting score for each stage and assigns it
* to exportFiles in the field corresponding to that stage's id
*/
$scope.rebuildCSV = function(scoreboard) {
$scope.csvdata = {};
$scope.csvname = {};
Object.keys(scoreboard).forEach(function(stageId) {
var ranking = scoreboard[stageId];
var rows = ranking.map(function(entry) {
return [
entry.rank,
entry.team.number,
entry.team.name,
entry.highest,
].concat(entry.scores);
$scope.buildExportFiles= function () {
Copy link
Collaborator

@poelstra poelstra Aug 28, 2017

Choose a reason for hiding this comment

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

rebuildCSV(scoreboard) gets passed in the scoreboard. This is intentional, because it may be that someone wanted to pass in e.g. a filtered version of the scoreboard.

(It did directly tweak things on $scope, which is bad, so probably it should have returned an object containing these items instead.)

Object.keys($scope.scoreboard).forEach(function (stageID) {
var teams = $scope.scoreboard[stageID];
teams = teams.map(function (teamEntry) {
return [teamEntry.rank, teamEntry.team.number,
teamEntry.team.name, teamEntry.highest].concat(teamEntry.scores);
});
var header = ["Rank", "Team Number", "Team Name", "Highest"];
var stage = $stages.get(stageId);
header = header.concat(stage.$rounds.map(function(round) { return "Round " + round; }));
rows.unshift(header);
$scope.csvname[stageId] = encodeURIComponent("ranking_" + stageId + ".csv");
$scope.csvdata[stageId] = "data:text/csv;charset=utf-8," + encodeURIComponent(toCSV(rows));
$scope.exportFiles[stageID] = "data:text/csv;charset=utf-8,"+encodeURIComponent($scope.encodeArray(teams));
});
};

// Rebuild CSV data and filenames when scoreboard is updated
$scope.$watch("scoreboard", function() {
$scope.rebuildCSV($scores.scoreboard);
$scope.$watch("scoreboard", function () {
$scope.buildExportFiles();
}, true);

$scope.$watchCollection("settings", function () {//we need to rebuild the files if the user changes his export format
$scope.buildExportFiles();
});

$settings.init().then(function () {//we have to wait for settings to initialize otherwise $scope.settings gets set to undefined
$scope.settings = $settings.settings;
});
$scope.stages = $stages.stages;
$scope.scoreboard = $scores.scoreboard;

$scope.getRoundLabel = function(round){
return "Round " + round;
};


}
]);
Expand Down
12 changes: 12 additions & 0 deletions src/js/views/scores.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,18 @@ define('views/scores',[
$scope.rev = (String($scope.sort) === String(col)) ? !$scope.rev : !!defaultSort;
$scope.sort = col;
};

$scope.sortIcon = function(col){
if(String($scope.sort)!== String(col)){//col and $scope.sort can be arrays, and so this is a quick and dirty way to check for equality
return '';
}
if ($scope.rev) {
return 'arrow_drop_down';
} else {
return 'arrow_drop_up';
}
};

$scope.removeScore = function(index) {
$scores.remove(index);
return $scores.save();
Expand Down
Loading