Rewrites checkVersion and checkVersionNewer in utils-service.js, refines upgradeToVersion

- Adds unit tests that cover all cases.
- Adds call to parseVersionString() so we can pass it version string e.g. '1.0.1'
- persistence-service.js now also manages JamstashVersion. It is set in localStorage from app.js's constant, and not from the changelog so we don't have to add a version to the changelog to actually do stuff.
- persistence-service.js's upgradeToVersion() now only applies newer changesets. This ensures we won't erase users' settings with changesets at every new version, e.g. resetting songSearchType to song every time.
- upgradeToVersion() will also notify the user of the upgrade, which was formerly done in settings.js

app/app.js

@@ -50,5 +50,5 @@ angular.module('JamStash', ['ngCookies', 'ngRoute', 'ngSanitize', 'ui.keypress',
     }]);
 }])
 
-//TODO: store as version object
+//TODO: Hyz: Fill with grunt task
 .constant('jamstashVersion', '4.4.5');

app/common/persistence-service.js

@@ -74,8 +74,7 @@ angular.module('jamstash.persistence', ['angular-locker',
 
     /* Manage user settings */
     this.getSettings = function () {
-        //TODO: store as version object
+        if(utils.checkVersionNewer(jamstashVersion, this.getVersion())) {
-        if(this.getVersion() !== jamstashVersion) {
             this.upgradeToVersion(jamstashVersion);
         }
         return locker.get('Settings');
@@ -91,25 +90,24 @@ angular.module('jamstash.persistence', ['angular-locker',
 
     /* Manage Jamstash Version */
     this.getVersion = function () {
-        return locker.get('version');
+        return locker.get('JamstashVersion');
     };
 
     this.upgradeToVersion = function (finalVersion) {
-        var currentVersion = utils.parseVersionString(this.getVersion());
+        var currentVersion = this.getVersion();
         var settings = locker.get('Settings');
-        // Apply all upgrades older than the final version
+        // Apply all upgrades older than the final version and newer than the current
-        // TODO: Hyz: Do not apply upgrades already applied (start from current version)
         var allUpgrades = _(jamstashVersionChangesets.versions).filter(function (toApply) {
-            //TODO: Hyz: have "checkVersion" do the conversion themselves
+            var olderOrEqualToFinal = utils.checkVersion(finalVersion, toApply.version);
-            var versionToCheck = utils.parseVersionString(toApply.version);
+            var newerThanCurrent = utils.checkVersionNewer(toApply.version, currentVersion);
-            var objFinalVersion = utils.parseVersionString(finalVersion);
+            return olderOrEqualToFinal && newerThanCurrent;
-            return utils.checkVersion(objFinalVersion, versionToCheck);
         });
         _(allUpgrades).each(function (versionUpg) {
             versionUpg.changeset(settings);
         });
         this.saveSettings(settings);
-        locker.put('version', finalVersion);
+        locker.put('JamstashVersion', finalVersion);
+        notifications.updateMessage('Version ' + currentVersion + ' to ' + finalVersion, true);
     };
 }])
 

app/common/persistence-service_test.js

@@ -12,6 +12,16 @@ describe("Persistence service", function() {
             });
             $provide.constant("jamstashVersion", "1.0.1");
             $provide.value("jamstashVersionChangesets", fakeVersionChangesets);
+
+            $provide.decorator('notifications', function () {
+                return jasmine.createSpyObj("notifications", ["updateMessage"]);
+            });
+
+            $provide.decorator('player', function () {
+                var fakePlayer = jasmine.createSpyObj("player", ["load", "addSongs"]);
+                fakePlayer.queue = [];
+                return fakePlayer;
+            });
         });
 
         inject(function (_persistence_, _player_, _notifications_, _locker_) {
@@ -27,8 +37,6 @@ describe("Persistence service", function() {
             artist: 'Isiah Hosfield',
             album: 'Tammanyize'
         };
-        player.queue = [];
-
         fakeStorage = {};
 
         locker.get.and.callFake(function(key) {
@@ -37,10 +45,6 @@ describe("Persistence service", function() {
     });
 
     describe("loadTrackPosition() -", function() {
-        beforeEach(function() {
-            spyOn(player, "load");
-        });
-
         it("Given a previously saved song in local storage, when I load the song, the player will load it", function() {
             fakeStorage = { 'CurrentSong': song };
 
@@ -69,8 +73,7 @@ describe("Persistence service", function() {
 
     describe("loadQueue() -", function() {
         beforeEach(function() {
-            spyOn(notifications, "updateMessage");
+            player.addSongs.and.callFake(function (songs) {
-            spyOn(player, "addSongs").and.callFake(function (songs) {
                 // Update the queue length so that notifications work
                 player.queue.length += songs.length;
             });
@@ -162,6 +165,7 @@ describe("Persistence service", function() {
         });
 
         it("Given that the previously stored Jamstash version was '1.0.0' and given the current constant jamstash.version was '1.0.1', when I get the settings, then upgradeToVersion will be called", function() {
+            fakeStorage.JamstashVersion = '1.0.0';
             spyOn(persistence, 'upgradeToVersion');
 
             persistence.getSettings();
@@ -193,18 +197,35 @@ describe("Persistence service", function() {
         expect(locker.forget).toHaveBeenCalledWith('Settings');
     });
 
+    describe("getVersion() -", function() {
+        it("Given a previously saved Jamstash version in local storage, it retrieves it", function() {
+            fakeStorage = { 'JamstashVersion': '1.2.5' };
+
+            var version = persistence.getVersion();
+
+            expect(locker.get).toHaveBeenCalledWith('JamstashVersion');
+            expect(version).toBe('1.2.5');
+        });
+
+        it("Given that no Jamstash version was previously saved in local storage, it returns undefined", function() {
+            var version = persistence.getVersion();
+
+            expect(locker.get).toHaveBeenCalledWith('JamstashVersion');
+            expect(version).toBeUndefined();
+        });
+    });
+
     describe("upgradeToVersion() -", function() {
-        describe("Given that Jamstash version '1.0.0' was previously stored in local storage,", function() {
+        it("Given that Jamstash version '1.0.0' was previously stored in local storage, when I upgrade the storage version to '1.0.1', Jamstash version '1.0.1' will be in local storage and the user will be notified", function() {
-            beforeEach(function() {
+            fakeStorage.JamstashVersion = '1.0.0';
-                fakeStorage.version = '1.0.0';
-            });
 
-            it("when I upgrade the storage version to '1.0.1', Jamstash version '1.0.1' will be in local storage", function() {
             persistence.upgradeToVersion('1.0.1');
-                expect(locker.put).toHaveBeenCalledWith('version', '1.0.1');
+
+            expect(locker.put).toHaveBeenCalledWith('JamstashVersion', '1.0.1');
+            expect(notifications.updateMessage).toHaveBeenCalledWith('Version 1.0.0 to 1.0.1', true);
         });
 
-            describe("that changesets for versions '1.0.1' and '1.0.2' were defined,", function() {
+        describe("Given that changesets for versions '1.0.1' and '1.0.2' were defined,", function() {
             beforeEach(function() {
                 fakeVersionChangesets.versions = [
                     {
@@ -236,6 +257,11 @@ describe("Persistence service", function() {
                 };
             });
 
+            describe("and that Jamstash version '1.0.0' was previously stored in local storage,", function() {
+                beforeEach(function() {
+                    fakeStorage.JamstashVersion = '1.0.0';
+                });
+
                  it("when I upgrade the storage version to '1.0.2', both changesets have been applied", function() {
                     persistence.upgradeToVersion('1.0.2');
                     expect(locker.put).toHaveBeenCalledWith('Settings', {
@@ -246,7 +272,7 @@ describe("Persistence service", function() {
                     });
                 });
 
-                it("when I upgrade the storage version to '1.0.1', only the first changeset has been applied", function() {
+                it("when I upgrade the storage version to '1.0.1', only the '1.0.1' changeset has been applied", function() {
                     persistence.upgradeToVersion('1.0.1');
                     expect(locker.put).toHaveBeenCalledWith('Settings', {
                         DefaultSearchType: 0,
@@ -259,6 +285,21 @@ describe("Persistence service", function() {
                     });
                 });
             });
+
+            it("and that Jamstash version '1.0.1' was previously stored in local storage, when I upgrade the storage version to '1.0.2', only the '1.0.2' changeset has been applied", function() {
+                fakeStorage.JamstashVersion = '1.0.1';
+
+                persistence.upgradeToVersion('1.0.2');
+                expect(locker.put).toHaveBeenCalledWith('Settings', {
+                    DefaultSearchType: {
+                        id: "song",
+                        name: "Song"
+                    },
+                    DefaultAlbumSort: 0,
+                    Username: "Hedrix",
+                    AutoPlay: true
+                });
+            });
         });
     });
 });

app/common/utils-service.js

@@ -235,6 +235,7 @@ angular.module('jamstash.utils', ['jamstash.settings.service'])
         var u = strurl.substring(0, strurl.indexOf('?'));
         return u;
     };
+
     this.parseVersionString = function (str) {
         if (typeof (str) !== 'string') { return false; }
         var x = str.split('.');
@@ -248,31 +249,21 @@ angular.module('jamstash.utils', ['jamstash.settings.service'])
             patch: pat
         };
     };
-    //TODO: Hyz: have "checkVersion" do the conversion themselves
+
-    this.checkVersion = function (runningVersion, minimumVersion) {
+    this.checkVersion = function (newerVersion, olderVersion) {
-        if (runningVersion.major >= minimumVersion.major) {
+        if (!angular.isObject(newerVersion)) {
-            if (runningVersion.minor >= minimumVersion.minor) {
+            newerVersion = this.parseVersionString(newerVersion);
-                if (runningVersion.patch >= minimumVersion.patch) {
+        }
+        if (!angular.isObject(olderVersion)) {
+            olderVersion = this.parseVersionString(olderVersion);
+        }
+        if (olderVersion.major !== undefined && newerVersion.major !== undefined && newerVersion.major > olderVersion.major) {
             return true;
         } else {
-                    return false;
+            if (olderVersion.minor !== undefined && newerVersion.minor !== undefined && newerVersion.minor > olderVersion.minor) {
-                }
-            } else {
-                return false;
-            }
-        } else {
-            return false;
-        }
-    };
-    //TODO: Hyz: have "checkVersion" do the conversion themselves
-    this.checkVersionNewer = function (runningVersion, newVersion) {
-        if (runningVersion.major < newVersion.major) {
                 return true;
             } else {
-            if (runningVersion.minor < newVersion.minor) {
+                if (olderVersion.patch !== undefined && newerVersion.patch !== undefined && newerVersion.patch >= olderVersion.patch) {
-                return true;
-            } else {
-                if (runningVersion.patch < newVersion.patch) {
                     return true;
                 } else {
                     return false;
@@ -280,6 +271,29 @@ angular.module('jamstash.utils', ['jamstash.settings.service'])
             }
         }
     };
+
+    this.checkVersionNewer = function (newerVersion, olderVersion) {
+        if (!angular.isObject(newerVersion)) {
+            newerVersion = this.parseVersionString(newerVersion);
+        }
+        if (!angular.isObject(olderVersion)) {
+            olderVersion = this.parseVersionString(olderVersion);
+        }
+        if (olderVersion.major !== undefined && newerVersion.major !== undefined && newerVersion.major > olderVersion.major) {
+            return true;
+        } else {
+            if (olderVersion.minor !== undefined && newerVersion.minor !== undefined && newerVersion.minor > olderVersion.minor) {
+                return true;
+            } else {
+                if (olderVersion.patch !== undefined && newerVersion.patch !== undefined && newerVersion.patch > olderVersion.patch) {
+                    return true;
+                } else {
+                    return false;
+                }
+            }
+        }
+    };
+
     this.reloadRoute = function (date) {
         if (reload) {
             $window.location.reload();

app/common/utils-service_test.js

@@ -1,5 +1,143 @@
 describe("utils service", function() {
     'use strict';
 
+    var $rootScope, utils, mockGlobals;
+    beforeEach(function() {
+        module('jamstash.utils', function ($provide) {
+            $provide.value('globals', mockGlobals);
+        });
 
+        inject(function (_utils_, _$rootScope_) {
+            utils = _utils_;
+            $rootScope = _$rootScope_;
+        });
+    });
+
+    describe("parseVersionString() -", function() {
+        it("Given a version string '2.0.1', when I parse it into a version object, then the result will be {major: 2, minor: 0, patch: 1}", function() {
+            var result = utils.parseVersionString('2.0.1');
+            expect(result).toEqual({major: 2, minor: 0, patch: 1});
+        });
+
+        it("Given a random string 'IHtd8EAL9HeLdc', when I parse it into a version object, then the result will be {major: 0, minor: 0, patch: 0}", function() {
+            var result = utils.parseVersionString('IHtd8EAL9HeLdc');
+            expect(result).toEqual({major: 0, minor: 0, patch: 0});
+        });
+
+        it("Given something other than a number, when I parse it into a version object, then the result will be false", function() {
+            var result = utils.parseVersionString(84.1061);
+            expect(result).toBeFalsy();
+        });
+    });
+
+    describe("checkVersion() -", function() {
+        var running, required;
+        beforeEach(function() {
+            running = '';
+            required = '';
+        });
+
+        it("Given two version strings '2.0.1' and '1.2.3', when I check the version required, the result will be true", function() {
+            running = '2.0.1';
+            required = '1.2.3';
+            expect(utils.checkVersion(running, required)).toBeTruthy();
+        });
+
+        it("Given two version objects {major: 2, minor: 0, patch: 1} and {major: 1, minor: 2, patch: 3}, when I check the version required, the result will be true", function() {
+            running = {
+                major: 2,
+                minor: 0,
+                patch: 1
+            };
+            required = {
+                major: 1,
+                minor: 2,
+                patch: 0
+            };
+            expect(utils.checkVersion(running, required)).toBeTruthy();
+        });
+
+        it("Given two version strings '1.3.0' and '1.2.3', when I check the version required, the result will be true", function() {
+            running = '1.3.0';
+            required = '1.2.3';
+            expect(utils.checkVersion(running, required)).toBeTruthy();
+        });
+
+        it("Given two version strings '1.2.2' and '1.2.3', when I check the version required, the result will be false", function() {
+            running = '1.2.2';
+            required = '1.2.3';
+            expect(utils.checkVersion(running, required)).toBeFalsy();
+        });
+
+        it("Given two version strings '1.2.3' and '1.2.3', when I check the version required, the result will be true", function() {
+            running = '1.2.3';
+            required = '1.2.3';
+            expect(utils.checkVersion(running, required)).toBeTruthy();
+        });
+
+        it("Given two random strings 'wISr91GRXzTsxkx' and 'uSIwvRDp8QJO', when I check the version required, the result will be true", function() {
+            running = 'wISr91GRXzTsxkx';
+            required = 'uSIwvRDp8QJO';
+            expect(utils.checkVersion(running, required)).toBeTruthy();
+        });
+    });
+
+    describe("checkVersionNewer() -", function() {
+        var newer, older;
+        beforeEach(function() {
+            newer = '';
+            older = '';
+        });
+
+        it("Given two version strings '2.0.1' and '1.2.3', when I check if the first version is newer, the result will be true", function() {
+            newer = '2.0.1';
+            older = '1.2.3';
+            expect(utils.checkVersionNewer(newer, older)).toBeTruthy();
+        });
+
+        it("Given two version objects {major: 2, minor: 0, patch: 1} and {major: 1, minor: 2, patch: 3}, when I check if the first version is newer, the result will be true", function() {
+            newer = {
+                major: 2,
+                minor: 0,
+                patch: 1
+            };
+            older = {
+                major: 1,
+                minor: 2,
+                patch: 0
+            };
+            expect(utils.checkVersionNewer(newer, older)).toBeTruthy();
+        });
+
+        it("Given two version strings '1.3.0' and '1.2.3', when I check if the first version is newer, the result will be true", function() {
+            newer = '1.3.0';
+            older = '1.2.3';
+            expect(utils.checkVersionNewer(newer, older)).toBeTruthy();
+        });
+
+        it("Given two version strings '1.2.2' and '1.2.3', when I check if the first version is newer, the result will be false", function() {
+            newer = '1.2.2';
+            older = '1.2.3';
+            expect(utils.checkVersionNewer(newer, older)).toBeFalsy();
+        });
+
+        it("Given two version strings '1.2.3' and '1.2.3', when I check if the first version is newer, the result will be false", function() {
+            newer = '1.2.3';
+            older = '1.2.3';
+            expect(utils.checkVersionNewer(newer, older)).toBeFalsy();
+        });
+
+        it("Given two version strings '1.0.1' and '1.0.0', when I check if the first version is newer, the result will be true", function() {
+            newer = '1.0.1';
+            older = '1.0.0';
+
+            expect(utils.checkVersionNewer(newer, older)).toBeTruthy();
+        });
+
+        it("Given two random strings 'wISr91GRXzTsxkx' and 'uSIwvRDp8QJO', when I check if the first version is newer, the result will be false", function() {
+            newer = 'wISr91GRXzTsxkx';
+            older = 'uSIwvRDp8QJO';
+            expect(utils.checkVersionNewer(newer, older)).toBeFalsy();
+        });
+    });
 });

app/settings/settings.js

@@ -86,15 +86,6 @@ angular.module('jamstash.settings.controller', ['jamstash.settings.service', 'ja
     json.getChangeLog(function (data) {
         $scope.changeLog = data.slice(0, 10);
         globals.ChangeLog = data;
-        var newVersion = $scope.changeLog[0].version;
-        if (!utils.getValue('JamstashVersion')) {
-            utils.setValue('JamstashVersion', newVersion);
-        }
-        var oldVersion = utils.getValue('JamstashVersion');
-        if (utils.checkVersionNewer(oldVersion, newVersion)) {
-            utils.setValue('JamstashVersion', newVersion);
-            notifications.updateMessage('Version ' + oldVersion + ' to ' + newVersion, true);
-        }
     });
     $scope.changeLogShowMore = function () {
         json.getChangeLog(function (data) {