Refactors the way music folders are handled

- Second part of the fix for tsquillario/Jamstash#212 - Fixes getIndexes
- Moved loading / saving / deleting the selected music folder to persistence-service.
- Moved the request to get all the music folders to subsonic-service.
- Moved SelectedMusicFolder to subsonic-controller's scope from the rootScope
- Fixes a bug where we would send a '-1' music folder id to Subsonic, causing an SQL error when choosing "All Folders" and refreshing the Artists list.
This commit is contained in:
Hyzual 2015-05-21 00:09:26 +02:00
parent 0bc1cea108
commit e43c4c7655
8 changed files with 294 additions and 71 deletions

View file

@ -10,7 +10,6 @@ angular.module('JamStash')
$rootScope.Genres = [];
$rootScope.Messages = [];
$rootScope.SelectedMusicFolder = "";
$rootScope.unity = null;
$scope.Page = Page;
$rootScope.loggedIn = function () {

View file

@ -77,6 +77,19 @@ angular.module('jamstash.persistence', ['angular-locker',
locker.forget('Volume');
};
/* Manage selected music folder */
this.getSelectedMusicFolder = function () {
return locker.get('MusicFolders');
};
this.saveSelectedMusicFolder = function (selectedMusicFolder) {
locker.put('MusicFolders', selectedMusicFolder);
};
this.deleteSelectedMusicFolder = function () {
locker.forget('MusicFolders');
};
/* Manage user settings */
this.getSettings = function () {
// If the latest version from changelog.json is newer than the version stored in local storage,

View file

@ -154,6 +154,48 @@ describe("Persistence service", function() {
expect(locker.forget).toHaveBeenCalledWith('Volume');
});
describe("getSelectedMusicFolder() -", function() {
it("Given a previously saved selected music folder in local storage, when I get the saved music folder, then an object containing the id and name of the selected music folder will be returned", function() {
fakeStorage = {
'MusicFolders': {
'id': 74,
'name': 'kooliman unhurled'
}
};
var selectedMusicFolder = persistence.getSelectedMusicFolder();
expect(locker.get).toHaveBeenCalledWith('MusicFolders');
expect(selectedMusicFolder).toEqual({
'id': 74,
'name': 'kooliman unhurled'
});
});
it("Given that no selected music folder was previously saved in local storage, when I get the saved music folder, then undefined will be returned", function() {
var selectedMusicFolder = persistence.getSelectedMusicFolder();
expect(locker.get).toHaveBeenCalledWith('MusicFolders');
expect(selectedMusicFolder).toBeUndefined();
});
});
it("saveSelectedMusicFolder() - given an object containing the id and name of the selected music folder, when I save the music folder, then it will be set in local storage", function() {
persistence.saveSelectedMusicFolder({
id: 41,
name: 'parlormaid carcinolytic'
});
expect(locker.put).toHaveBeenCalledWith('MusicFolders', {
id: 41,
name: 'parlormaid carcinolytic'
});
});
it("deleteSelectedMusicFolder() - when I delete the selected music folder, then it will be erased from local storage", function() {
persistence.deleteSelectedMusicFolder();
expect(locker.forget).toHaveBeenCalledWith('MusicFolders');
});
describe("getSettings() -", function() {
beforeEach(function() {
spyOn(persistence, 'upgradeVersion');

View file

@ -34,6 +34,7 @@ angular.module('jamstash.subsonic.service', ['angular-underscore/utils',
var subsonicService = {
showIndex: $rootScope.showIndex,
showPlaylist: showPlaylist,
//TODO: Hyz: Do we still need this ? it's only used in the songpreview directive
getSongTemplate: function (callback) {
var id = '16608';
var url = globals.BaseURL() + '/getMusicDirectory.view?' + globals.BaseParams() + '&id=' + id;
@ -121,13 +122,21 @@ angular.module('jamstash.subsonic.service', ['angular-underscore/utils',
return subsonicService.subsonicRequest('ping.view');
},
getMusicFolders: function () {
var exception = {reason: 'No music folder found on the Subsonic server.'};
var promise = subsonicService.subsonicRequest('getMusicFolders.view')
.then(function (subsonicResponse) {
if (subsonicResponse.musicFolders !== undefined && subsonicResponse.musicFolders.musicFolder !== undefined) {
return [].concat(subsonicResponse.musicFolders.musicFolder);
} else {
return $q.reject(exception);
}
});
return promise;
},
getArtists: function (folder) {
var exception = {reason: 'No artist found on the Subsonic server.'};
// TODO: Hyz: Move loading / saving the music folder to persistence-service
if (isNaN(folder) && utils.getValue('MusicFolders')) {
var musicFolder = angular.fromJson(utils.getValue('MusicFolders'));
folder = musicFolder.id;
}
var params;
if (!isNaN(folder)) {
params = {

View file

@ -1,7 +1,7 @@
describe("Subsonic service -", function() {
'use strict';
var subsonic, mockBackend, mockGlobals, $q,
var $q, mockBackend, subsonic, mockGlobals,
response, url;
beforeEach(function() {
// We redefine it because in some tests we need to alter the settings
@ -624,6 +624,55 @@ describe("Subsonic service -", function() {
});
});
describe("getMusicFolders() -", function() {
beforeEach(function() {
url = 'http://demo.subsonic.com/rest/getMusicFolders.view?'+
'c=Jamstash&callback=JSON_CALLBACK&f=jsonp&p=enc:cGFzc3dvcmQ%3D&u=Hyzual&v=1.10.2';
});
it("Given that there were 2 music folders in my library, when I get the music folders, then a promise will be resolved with an array of 2 music folders", function() {
response["subsonic-response"].musicFolders = {
musicFolder: [
{ id: 80, name: "languageless" },
{ id: 38, name: "mala" }
]
};
mockBackend.expectJSONP(url).respond(JSON.stringify(response));
var promise = subsonic.getMusicFolders();
mockBackend.flush();
expect(promise).toBeResolvedWith([
{ id: 80, name: "languageless" },
{ id: 38, name: "mala" }
]);
});
it("Given that there was 1 music folder in my Madsonic library, when I get the music folders, then a promise will be resolved with an array of 1 music folder", function() {
response["subsonic-response"].musicFolders = {
musicFolder: {id: 56, name: "dite"}
};
mockBackend.expectJSONP(url).respond(JSON.stringify(response));
var promise = subsonic.getMusicFolders();
mockBackend.flush();
expect(promise).toBeResolvedWith([
{ id: 56, name: "dite"}
]);
});
it("Given that there wasn't any music folder in my library, when I get the music folders, then a promise will be rejected with an error message", function() {
response["subsonic-response"].musicFolders = {};
mockBackend.expectJSONP(url).respond(JSON.stringify(response));
var promise = subsonic.getMusicFolders();
mockBackend.flush();
expect(promise).toBeRejectedWith({reason: 'No music folder found on the Subsonic server.'});
});
});
describe("getArtists() -", function() {
beforeEach(function() {
url = 'http://demo.subsonic.com/rest/getIndexes.view?'+
@ -658,6 +707,15 @@ describe("Subsonic service -", function() {
expect(promise).toBeResolvedWith(response["subsonic-response"].indexes);
});
it("Given a folder id, when I get the artists, then it will be used as parameter in the request", function() {
url = 'http://demo.subsonic.com/rest/getIndexes.view?'+
'c=Jamstash&callback=JSON_CALLBACK&f=jsonp'+'&musicFolderId=54'+'&p=enc:cGFzc3dvcmQ%3D&u=Hyzual&v=1.10.2';
mockBackend.expectJSONP(url).respond(JSON.stringify(response));
subsonic.getArtists(54);
mockBackend.flush();
});
it("Given that there were 2 artist at the top level of my Madsonic library, when I get the artists, then a promise will be resolved with an array of two artist", function() {
response["subsonic-response"].indexes = {
shortcut: { id: 433, name: "Podcast" },

View file

@ -86,7 +86,9 @@
</ul>
<div class="clear"></div>
<div id="IndexContainer" class="leftsubsection" ng-show="showIndex">
<select id="MusicFolders" class="folders" ng-model="$root.SelectedMusicFolder" ng-options="o.name for o in MusicFolders"></select>
<select id="MusicFolders" class="folders" ng-model="SelectedMusicFolder" ng-options="o.name for o in MusicFolders track by o.id">
<option value="">All Folders</option>
</select>
<div id="AZIndex" ng-show="!settings.HideAZ" class="subactionsfixed">
<a href="" ng-click="toggleAZ()" stop-event="click">A-Z</a>
</div>

View file

@ -4,10 +4,10 @@
* Access and use the Subsonic Server. The Controller is in charge of relaying the Service's messages to the user through the
* notifications.
*/
angular.module('jamstash.subsonic.controller', ['jamstash.subsonic.service', 'jamstash.player.service'])
angular.module('jamstash.subsonic.controller', ['jamstash.subsonic.service', 'jamstash.player.service', 'jamstash.persistence'])
.controller('SubsonicController', ['$scope', '$rootScope', '$routeParams', '$window', 'utils', 'globals', 'map', 'subsonic', 'notifications', 'player',
function ($scope, $rootScope, $routeParams, $window, utils, globals, map, subsonic, notifications, player) {
.controller('SubsonicController', ['$scope', '$rootScope', '$routeParams', '$window', 'utils', 'globals', 'map', 'subsonic', 'notifications', 'player', 'persistence',
function ($scope, $rootScope, $routeParams, $window, utils, globals, map, subsonic, notifications, player, persistence) {
'use strict';
$scope.settings = globals.settings;
@ -141,13 +141,20 @@ angular.module('jamstash.subsonic.controller', ['jamstash.subsonic.service', 'ja
}
}
});
$rootScope.$watch("SelectedMusicFolder", function (newValue, oldValue) {
$scope.$watch("SelectedMusicFolder", function (newValue, oldValue) {
if (newValue !== oldValue) {
// TODO: Hyz: Move loading / saving the music folder to persistence-service
utils.setValue('MusicFolders', angular.toJson(newValue), true);
$scope.getArtists(newValue.id);
var folderId;
if (newValue) {
folderId = newValue.id;
persistence.saveSelectedMusicFolder(newValue);
} else {
persistence.deleteSelectedMusicFolder();
}
$scope.getArtists(folderId);
}
});
$scope.searching = {
query: "",
typeId: globals.settings.DefaultSearchType,
@ -201,7 +208,12 @@ angular.module('jamstash.subsonic.controller', ['jamstash.subsonic.service', 'ja
$scope.song = data.song;
});
};
$scope.getArtists = function (folder) {
var savedFolder = $scope.SelectedMusicFolder;
if (isNaN(folder) && savedFolder) {
folder = savedFolder.id;
}
var promise = subsonic.getArtists(folder);
$scope.handleErrors(promise).then(function (data) {
$scope.index = data.index;
@ -214,8 +226,9 @@ angular.module('jamstash.subsonic.controller', ['jamstash.subsonic.service', 'ja
}
});
};
$scope.refreshArtists = function () {
utils.setValue('MusicFolders', null, true);
$scope.SelectedMusicFolder = undefined;
$scope.getArtists();
$scope.getPlaylists();
};
@ -594,43 +607,19 @@ angular.module('jamstash.subsonic.controller', ['jamstash.subsonic.service', 'ja
};
$scope.getMusicFolders = function () {
$.ajax({
url: globals.BaseURL() + '/getMusicFolders.view?' + globals.BaseParams(),
method: 'GET',
dataType: globals.settings.Protocol,
timeout: globals.settings.Timeout,
success: function (data) {
if (data["subsonic-response"].musicFolders.musicFolder !== undefined) {
var folders = [];
if (data["subsonic-response"].musicFolders.musicFolder.length > 0) {
folders = data["subsonic-response"].musicFolders.musicFolder;
} else {
folders[0] = data["subsonic-response"].musicFolders.musicFolder;
}
folders.unshift({
"id": -1,
"name": "All Folders"
});
$rootScope.MusicFolders = folders;
if (utils.getValue('MusicFolders')) {
var folder = angular.fromJson(utils.getValue('MusicFolders'));
var i = 0, index = "";
angular.forEach($rootScope.MusicFolders, function (item, key) {
if (item.id == folder.id) {
index = i;
}
i++;
});
$rootScope.SelectedMusicFolder = $rootScope.MusicFolders[index];
} else {
$rootScope.SelectedMusicFolder = $rootScope.MusicFolders[0];
}
$scope.$apply();
var promise = subsonic.getMusicFolders();
$scope.handleErrors(promise).then(function (musicFolders) {
var folders = musicFolders;
$scope.MusicFolders = folders;
var savedFolder = persistence.getSelectedMusicFolder();
if (savedFolder) {
if (_.findIndex(folders, {id: savedFolder.id}) !== -1) {
$scope.SelectedMusicFolder = savedFolder;
}
}
});
};
/**
* Change the order of playlists through jQuery UI's sortable
*/
@ -650,12 +639,12 @@ angular.module('jamstash.subsonic.controller', ['jamstash.subsonic.service', 'ja
};
/* Launch on Startup */
$scope.getMusicFolders();
$scope.getArtists();
$scope.getPlaylists();
$scope.getGenres();
$scope.getPodcasts();
$scope.openDefaultSection();
$scope.getMusicFolders();
if ($routeParams.artistId && $routeParams.albumId) {
$scope.getAlbumByTag($routeParams.albumId);
} else if ($routeParams.artistId) {

View file

@ -2,14 +2,14 @@ describe("Subsonic controller", function() {
'use strict';
var scope, $rootScope, $controller, $window,
subsonic, notifications, player, controllerParams, deferred;
subsonic, notifications, player, utils, persistence, controllerParams, deferred;
beforeEach(function() {
jasmine.addCustomEqualityTester(angular.equals);
module('jamstash.subsonic.controller');
inject(function (_$controller_, _$rootScope_, utils, globals, map, $q) {
inject(function (_$controller_, _$rootScope_, globals, map, $q) {
$rootScope = _$rootScope_;
scope = $rootScope.$new();
deferred = $q.defer();
@ -19,34 +19,42 @@ describe("Subsonic controller", function() {
"confirm"
]);
notifications = jasmine.createSpyObj("notifications", ["updateMessage"]);
utils = jasmine.createSpyObj("utils", ["getValue"]);
persistence = jasmine.createSpyObj("persistence", [
"getSelectedMusicFolder",
"saveSelectedMusicFolder",
"deleteSelectedMusicFolder"
]);
// Mock the subsonic service
subsonic = jasmine.createSpyObj("subsonic", [
"getSongs",
"recursiveGetSongs",
"deletePlaylist",
"getAlbums",
"getArtists",
"getGenres",
"getPlaylists",
"getPodcasts",
"getRandomStarredSongs",
"getRandomSongs",
"getMusicFolders",
"getPlaylist",
"newPlaylist",
"deletePlaylist",
"savePlaylist",
"getPlaylists",
"getPodcast",
"search"
"getPodcasts",
"getRandomSongs",
"getRandomStarredSongs",
"getSongs",
"newPlaylist",
"recursiveGetSongs",
"savePlaylist",
"search",
]);
// We make them return different promises and use our deferred variable only when testing
// a particular function, so that they stay isolated
subsonic.getSongs.and.returnValue($q.defer().promise);
subsonic.recursiveGetSongs.and.returnValue($q.defer().promise);
subsonic.getAlbums.and.returnValue($q.defer().promise);
subsonic.getArtists.and.returnValue($q.defer().promise);
subsonic.getGenres.and.returnValue($q.defer().promise);
subsonic.getMusicFolders.and.returnValue($q.defer().promise);
subsonic.getPlaylists.and.returnValue($q.defer().promise);
subsonic.getPodcasts.and.returnValue($q.defer().promise);
subsonic.getSongs.and.returnValue($q.defer().promise);
subsonic.recursiveGetSongs.and.returnValue($q.defer().promise);
subsonic.showIndex = false;
// Mock the player service
@ -73,7 +81,8 @@ describe("Subsonic controller", function() {
map: map,
subsonic: subsonic,
notifications: notifications,
player: player
player: player,
persistence: persistence
};
});
});
@ -82,6 +91,7 @@ describe("Subsonic controller", function() {
beforeEach(function() {
$controller('SubsonicController', controllerParams);
scope.selectedPlaylist = null;
scope.$apply();
});
describe("getSongs -", function() {
@ -513,12 +523,36 @@ describe("Subsonic controller", function() {
//TODO: Hyz: all starred
describe("When I load the artists,", function() {
describe("", function() {
beforeEach(function() {
spyOn(scope, "getArtists");
});
it("Given no previously selected music folder, when I select a music folder, then it will be stored in persistence and the artists will be loaded from subsonic", function() {
scope.SelectedMusicFolder = { id: 22, name: "Cascadia" };
scope.$apply();
expect(persistence.saveSelectedMusicFolder).toHaveBeenCalledWith({ id: 22, name: "Cascadia" });
expect(scope.getArtists).toHaveBeenCalledWith(22);
});
it("Given a previously selected music folder, when I select the 'All Folders' (undefined) music folder, then the stored value will be deleted from persistence and all the artists will be loaded from subsonic", function() {
scope.SelectedMusicFolder = { id: 23, name: "grantable" };
scope.$apply();
scope.SelectedMusicFolder = undefined;
scope.$apply();
expect(persistence.deleteSelectedMusicFolder).toHaveBeenCalled();
expect(scope.getArtists).not.toHaveBeenCalledWith(jasmine.any(Number));
});
});
describe("getArtists() -", function() {
beforeEach(function() {
subsonic.getArtists.and.returnValue(deferred.promise);
});
it("Given that there are songs in the library, it loads the artists and publishes them to the scope", function() {
it("Given that there were artists in the library, when I load the artists, then subsonic will be queried an index array containing the artists and a shortcut array containing the shortcuts (such as Podcasts) will be publisehd to the scope", function() {
scope.getArtists();
deferred.resolve({
index: [
@ -539,7 +573,15 @@ describe("Subsonic controller", function() {
]);
});
it("Given that there aren't any songs in the library, when loading indexes, it notifies the user with an error message", function() {
it("Given no folder id and given a selected music folder had been set in the scope, when I get the artists, then the selected music folder's id will be used as parameter to subsonic service", function() {
scope.SelectedMusicFolder = { id: 62, name: "dollardee" };
scope.getArtists();
expect(subsonic.getArtists).toHaveBeenCalledWith(62);
});
it("Given that there weren't any artist in the library, when I load the artists, then a notification with an error message will be displayed", function() {
scope.getArtists();
deferred.reject({reason: 'No artist found on the Subsonic server.'});
scope.$apply();
@ -549,13 +591,24 @@ describe("Subsonic controller", function() {
expect(notifications.updateMessage).toHaveBeenCalledWith('No artist found on the Subsonic server.', true);
});
it("it lets handleErrors handle HTTP and Subsonic errors", function() {
it("Given that the server was unreachable, when I get the music folders, then handleErrors() will deal with the error", function() {
spyOn(scope, 'handleErrors').and.returnValue(deferred.promise);
scope.getArtists();
expect(scope.handleErrors).toHaveBeenCalledWith(deferred.promise);
});
});
it("refreshArtists() - When I refresh the artists, then the selected music folder will be reset to undefined and the artists and playlists will be loaded", function() {
spyOn(scope, "getArtists");
spyOn(scope, "getPlaylists");
scope.refreshArtists();
expect(scope.SelectedMusicFolder).toBeUndefined();
expect(scope.getArtists).toHaveBeenCalled();
expect(scope.getPlaylists).toHaveBeenCalled();
});
describe("When I load the playlists,", function() {
beforeEach(function() {
subsonic.getPlaylists.and.returnValue(deferred.promise);
@ -631,7 +684,7 @@ describe("Subsonic controller", function() {
expect(scope.getPlaylists).toHaveBeenCalled();
});
it("Given no selected playlist, when I try to delete a playlist, an error message will be notified", function() {
it("Given no selected playlist, when I try to delete a playlist, an error notification will be displayed", function() {
scope.selectedPlaylist = null;
scope.deletePlaylist();
@ -663,7 +716,7 @@ describe("Subsonic controller", function() {
expect(notifications.updateMessage).toHaveBeenCalledWith('Playlist Updated!', true);
});
it("Given no selected playlist, when I try to save a playlist, an error message will be notified", function() {
it("Given no selected playlist, when I try to save a playlist, an error notification will be displayed", function() {
scope.selectedPlaylist = null;
scope.savePlaylist();
@ -705,6 +758,64 @@ describe("Subsonic controller", function() {
});
});
describe("getMusicFolders", function() {
beforeEach(function() {
subsonic.getMusicFolders.and.returnValue(deferred.promise);
});
it("Given that there were music folders in the library, when I get the music folders, then the folders will be published to the scope", function() {
scope.getMusicFolders();
deferred.resolve([
{ id: 74, name: "scirrhosis"},
{ id: 81, name: "drooper"}
]);
scope.$apply();
expect(subsonic.getMusicFolders).toHaveBeenCalled();
expect(scope.MusicFolders).toEqual([
{ id: 74, name: "scirrhosis"},
{ id: 81, name: "drooper"}
]);
});
describe("Given that there was a selected music folder previously saved in persistence", function() {
it("and that it matched one of the music folders returned by subsonic, when I get the music folders, then the scope's selected music folder will be set", function() {
persistence.getSelectedMusicFolder.and.returnValue({ id: 79, name: "dismember" });
scope.getMusicFolders();
deferred.resolve([
{ id: 93, name: "illish" },
{ id: 79, name: "dismember" }
]);
scope.$apply();
expect(scope.SelectedMusicFolder).toEqual({ id: 79, name: "dismember" });
});
it("and that it didn't match one of the music folders returned by subsonic, when I get the music folders, then the scope's selected music folder will be undefined", function() {
persistence.getSelectedMusicFolder.and.returnValue({ id: 49, name: "metafulminuric" });
scope.getMusicFolders();
deferred.resolve([
{ id: 94, name: "dorsiflexor" }
]);
scope.$apply();
expect(scope.SelectedMusicFolder).toBeUndefined();
});
});
it("Given that there weren't any music folder in the library, when I get the music folders, then handleErrors() will deal with the error", function() {
spyOn(scope, 'handleErrors').and.returnValue(deferred.promise);
scope.getMusicFolders();
deferred.reject({reason: 'No music folder found on the Subsonic server.'});
scope.$apply();
expect(scope.handleErrors).toHaveBeenCalledWith(deferred.promise);
});
});
describe("search() -", function() {
beforeEach(function() {
subsonic.search.and.returnValue(deferred.promise);