From 3aab5c64dfc61bbe7d0a6896a6f4a642815aab10 Mon Sep 17 00:00:00 2001 From: Martin Poelstra Date: Sat, 16 Sep 2017 01:51:05 +0200 Subject: [PATCH 01/38] scores: Move edit functions to group them, no functional changes. --- spec/views/scoresSpec.js | 32 ++++++++++++++++---------------- src/js/views/scores.js | 27 ++++++++++++++------------- 2 files changed, 30 insertions(+), 29 deletions(-) diff --git a/spec/views/scoresSpec.js b/spec/views/scoresSpec.js index 19d97c6f..a7c1e9b7 100644 --- a/spec/views/scoresSpec.js +++ b/spec/views/scoresSpec.js @@ -71,22 +71,6 @@ describe('scores', function() { }); }); - describe('publishScore',function() { - it('should publish a score and save it',function() { - $scope.publishScore(0); - expect(scoresMock.update).toHaveBeenCalledWith(0, {score: 1, index: 0, published: true}); - expect(scoresMock.save).toHaveBeenCalled(); - }); - }); - - describe('unpublishScore',function() { - it('should unpublish a score and save it',function() { - $scope.unpublishScore(0); - expect(scoresMock.update).toHaveBeenCalledWith(0, {score: 1, index: 0, published: false}); - expect(scoresMock.save).toHaveBeenCalled(); - }); - }); - describe('finishEditScore',function() { it('should call update and save',function() { $scope.editScore(0); @@ -110,6 +94,22 @@ describe('scores', function() { }); }); + describe('publishScore',function() { + it('should publish a score and save it',function() { + $scope.publishScore(0); + expect(scoresMock.update).toHaveBeenCalledWith(0, {score: 1, index: 0, published: true}); + expect(scoresMock.save).toHaveBeenCalled(); + }); + }); + + describe('unpublishScore',function() { + it('should unpublish a score and save it',function() { + $scope.unpublishScore(0); + expect(scoresMock.update).toHaveBeenCalledWith(0, {score: 1, index: 0, published: false}); + expect(scoresMock.save).toHaveBeenCalled(); + }); + }); + describe('pollSheets',function() { xit('should call pollSheets of scores',function() { $scope.pollSheets(); diff --git a/src/js/views/scores.js b/src/js/views/scores.js index 574ab79c..355bb665 100644 --- a/src/js/views/scores.js +++ b/src/js/views/scores.js @@ -24,29 +24,34 @@ define('views/scores',[ $scores.remove(index); return $scores.save(); }; + $scope.editScore = function(index) { var score = $scores.scores[index]; score.$editing = true; }; - $scope.publishScore = function(index) { + $scope.finishEditScore = function (index) { + // The score entry is edited 'inline', then used to + // replace the entry in the scores list and its storage. + // Because scores are always 'sanitized' before storing, + // the $editing flag is automatically discarded. var score = $scores.scores[index]; - score.published = true; saveScore(score); }; - $scope.unpublishScore = function(index) { + $scope.cancelEditScore = function () { + $scores._update(); + }; + + $scope.publishScore = function(index) { var score = $scores.scores[index]; - score.published = false; + score.published = true; saveScore(score); }; - $scope.finishEditScore = function(index) { - // The score entry is edited 'inline', then used to - // replace the entry in the scores list and its storage. - // Because scores are always 'sanitized' before storing, - // the $editing flag is automatically discarded. + $scope.unpublishScore = function(index) { var score = $scores.scores[index]; + score.published = false; saveScore(score); }; @@ -59,10 +64,6 @@ define('views/scores',[ } } - $scope.cancelEditScore = function() { - $scores._update(); - }; - $scope.pollSheets = function() { return $scores.pollSheets().catch(function(err) { log("pollSheets() failed", err); From 05e76945338cca9d5fafef5ee67bfb5aa079b1c8 Mon Sep 17 00:00:00 2001 From: Martin Poelstra Date: Sat, 16 Sep 2017 09:16:37 +0200 Subject: [PATCH 02/38] spec/scores: Allow changing mock scores without breaking unrelated tests. --- spec/views/scoresSpec.js | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/spec/views/scoresSpec.js b/spec/views/scoresSpec.js index a7c1e9b7..e7c14397 100644 --- a/spec/views/scoresSpec.js +++ b/spec/views/scoresSpec.js @@ -5,6 +5,7 @@ describe('scores', function() { }); var $scope, controller, scoresMock, teamsMock, stagesMock,$window,$q; + var originalMockScores; beforeEach(function() { angular.mock.module(module.name); @@ -21,6 +22,7 @@ describe('scores', function() { '$teams': teamsMock, '$stages': stagesMock, }); + originalMockScores = angular.copy(scoresMock.scores); }); $window.alert = jasmine.createSpy('alertSpy'); }); @@ -97,7 +99,9 @@ describe('scores', function() { describe('publishScore',function() { it('should publish a score and save it',function() { $scope.publishScore(0); - expect(scoresMock.update).toHaveBeenCalledWith(0, {score: 1, index: 0, published: true}); + var expectedScore = angular.copy(originalMockScores[0]); + expectedScore.published = true; + expect(scoresMock.update).toHaveBeenCalledWith(0, expectedScore); expect(scoresMock.save).toHaveBeenCalled(); }); }); @@ -105,7 +109,7 @@ describe('scores', function() { describe('unpublishScore',function() { it('should unpublish a score and save it',function() { $scope.unpublishScore(0); - expect(scoresMock.update).toHaveBeenCalledWith(0, {score: 1, index: 0, published: false}); + expect(scoresMock.update).toHaveBeenCalledWith(0, originalMockScores[0]); expect(scoresMock.save).toHaveBeenCalled(); }); }); From 540940997481644de8722f818461455f5a49bc6b Mon Sep 17 00:00:00 2001 From: Martin Poelstra Date: Sat, 16 Sep 2017 09:17:24 +0200 Subject: [PATCH 03/38] spec/scores: More realistic mock scores for testing score edit. --- spec/mocks/scoresMock.js | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/spec/mocks/scoresMock.js b/spec/mocks/scoresMock.js index c9002e76..06e1f670 100644 --- a/spec/mocks/scoresMock.js +++ b/spec/mocks/scoresMock.js @@ -2,10 +2,20 @@ function createScoresMock($q,scoreboard) { scoreboard = scoreboard || {}; return { scores: [{ + stageId: "qualifing", + round: 2, + teamNumber: 10, + table: "table 1", score: 1, + published: false, index: 0 },{ + stageId: "final", + round: 1, + teamNumber: 20, + table: "table 2", score: 2, + published: false, index: 1 }], scoreboard: scoreboard, From 8b966e220a50d2123150327701be801a2617f0ed Mon Sep 17 00:00:00 2001 From: Martin Poelstra Date: Sat, 16 Sep 2017 09:22:25 +0200 Subject: [PATCH 04/38] scores: Change score editing to use shadow state, allow seamless server-updates of non-edited scores. --- src/js/views/scores.js | 76 ++++++++++++++++++++++++++++++++----- src/views/pages/scores.html | 38 +++++++++---------- 2 files changed, 85 insertions(+), 29 deletions(-) diff --git a/src/js/views/scores.js b/src/js/views/scores.js index 355bb665..96e7763e 100644 --- a/src/js/views/scores.js +++ b/src/js/views/scores.js @@ -16,10 +16,58 @@ define('views/scores',[ $scope.scores = $scores.scores; $scope.stages = $stages.stages; + $scope.editing = {}; // Keep state of currently-editing scores + $scope.original = {}; // Keep original score when edit started + + var editableProperties = ["stageId", "round", "table", "teamNumber", "score", "published"]; + + /** + * Pick only the properties of a score that we need while editing + * and for determining whether a score is modified (on the server) + * while we were editing it (i.e. to cancel the edit if needed). + * @param score A single score object from ng-scores + * @return A new object with only properties specified in `editableProperties` + * copied from `score` + */ + function scoreToEditState(score) { + // Note that we don't use e.g. angular.copy(), because scores + // may have subtle changes in sub-objects (e.g. $$-stuff in a + // stage), that we don't care about for checking whether we want + // to cancel the edit. + // Also, scores are often 'polluted' with an extra `index` + // property, which typically doesn't exist yet on 'fresh' scores + // from the server. + var editState = {}; + editableProperties.forEach(function (prop) { + editState[prop] = score[prop]; + }); + return editState; + } + + // Watch for new scores. If they change, keep any edit mode + // as long as the score didn't change since the edit was started. + // Otherwise, cancel the edit. + $scope.$watch("scores", function (newScores) { + var indexes = Object.keys($scope.editing); + indexes.forEach(function(index) { + // Cancel edit mode if the new server value is different + // than what we had before we started editing. + // Note that this also cancels the edit when the item is + // now deleted. + var ourOriginal = $scope.original[index]; + var newScore = scoreToEditState(newScores[index]); + if (!angular.equals(ourOriginal, newScore)) { + delete $scope.editing[index]; + delete $scope.original[index]; + } + }); + }, true); + $scope.doSort = function(col, defaultSort) { $scope.rev = (String($scope.sort) === String(col)) ? !$scope.rev : !!defaultSort; $scope.sort = col; }; + $scope.removeScore = function(index) { $scores.remove(index); return $scores.save(); @@ -27,20 +75,28 @@ define('views/scores',[ $scope.editScore = function(index) { var score = $scores.scores[index]; - score.$editing = true; + // Create two copies of the score: one to store the unsaved edit, + // and another one to compare whether a new version of the + // server scores changed compare to what we have. + $scope.editing[index] = angular.copy(score); + $scope.original[index] = scoreToEditState(score); }; - $scope.finishEditScore = function (index) { - // The score entry is edited 'inline', then used to - // replace the entry in the scores list and its storage. - // Because scores are always 'sanitized' before storing, - // the $editing flag is automatically discarded. - var score = $scores.scores[index]; - saveScore(score); + $scope.finishEditScore = function(index) { + // Merge our edit state back into the current + // score entry (in case the entry was recently + // updated on the server). + // Prevents accidentally 'resetting' a property + // that isn't listed in scoreToEditState(). + var merged = angular.extend({}, $scores.scores[index], $scope.editing[index]); + delete $scope.editing[index]; + delete $scope.original[index]; + saveScore(merged); }; - $scope.cancelEditScore = function () { - $scores._update(); + $scope.cancelEditScore = function(index) { + delete $scope.editing[index]; + delete $scope.original[index]; }; $scope.publishScore = function(index) { diff --git a/src/views/pages/scores.html b/src/views/pages/scores.html index 0e21587c..2338b3f7 100644 --- a/src/views/pages/scores.html +++ b/src/views/pages/scores.html @@ -29,47 +29,47 @@

{{score.index + 1}} error - + {{score.stage.name}} - - + + error - + {{score.round}} - - + + - + {{score.table}} - - + + error - + {{score.team.number}} - - + + {{score.team.name}} error - + {{score.score}} - - + + error @@ -87,27 +87,27 @@

Unpublish