Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Amplitude.getSongsInPlaylist returning wrong songs in array #265

Closed
troubleshooter opened this issue May 21, 2018 · 3 comments
Closed

Amplitude.getSongsInPlaylist returning wrong songs in array #265

troubleshooter opened this issue May 21, 2018 · 3 comments
Assignees
Labels
Bug: Needs Confirmation 🧐 Bugs that are reported, but needs to be confirmed and replicated.

Comments

@troubleshooter
Copy link

Environment

  • Browser: Chrome
  • Browser Version: 66.0.3359.181 (Official Build) (64-bit)
  • Operating System: Mac OS X
  • Amplitude.js Version: 3.2.3

Issue description

Note: I corrected the typo in amplitude.js per issue #230 first.

My init has 25 songs in it with two playlists. The first playlist is composed of 12 songs, with song indexes 0 to 11. The second playlist is composed of 13 songs, with song indexes from 12 to 24.

Calling Amplitude.getSongsInPlaylist("first_playlist") returns array with 12 entries corresponding to the songs at indexes 0 to 11. No problem here.

Calling Amplitude.getSongsInPlaylist("second_playlist") returns array with 13 entries. While the count of the songs is correct, the array returns the songs at indexes 0 to 12 rather than 12 to 24.

Steps to reproduce the issue

  1. Call Amplitude.getSongsInPlaylist("first_playlist")
  2. Returns correct array.
  3. Call Amplitude.getSongsInPlaylist("second_playlist")
  4. Returns incorrect songs.

What is expected?

I'd expect an array returned listing songs from the second playlist.

Link to where issue can be reproduced

http://test.gracemenagerie.com/music2.html

Additional details / screenshots

Note: I corrected the typo in amplitude.js per issue #230.

@troubleshooter
Copy link
Author

I've fixed it. I've no real knowledge of javascript so perhaps there's a better way. Demo: https://test/gracemenagerie.com/music4.html

The function as it currently is in repo:

	/*--------------------------------------------------------------------------
 	Gets all of the songs in a playlist
 		Public Accessor: Amplitude.getSongsInPlaylist( playlist );
 		@param 	string 	playlist The playlist key
 --------------------------------------------------------------------------*/
	function getSongsInPlaylist(playlist) {
		var songsArray = [];

		for (var i = 0; i < _config2.default.playlist[playlist].length; i++) {
			songsArray.push(_config2.default.songs[i]);
		}

		return songsArray;
	}

The fixed function:


	function getSongsInPlaylist(playlist) {
		var songsArray = [];

		for (var i = 0; i < _config2.default.playlists[playlist].length; i++) {
		        var songIndex = _config2.default.playlists[playlist][i];
			songsArray.push(_config2.default.songs[songIndex]);
		}

		return songsArray;
	}

@jaydrogers jaydrogers added the Bug: Needs Confirmation 🧐 Bugs that are reported, but needs to be confirmed and replicated. label Jun 19, 2018
@jaydrogers
Copy link
Member

We'll be considering this in v3.4. Thanks for adding all of the detail for this.

@danpastori
Copy link
Contributor

@troubleshooter Thanks for pointing this out. I have this fixed in dev now from the method that you provided. This works now! See: https://github.com/521dimensions/amplitudejs/tree/dev

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug: Needs Confirmation 🧐 Bugs that are reported, but needs to be confirmed and replicated.
Projects
None yet
Development

No branches or pull requests

3 participants