search

RocketMan / zookeeper

Zookeeper Online is a music database and charting application for student and independent radio.
GNU General Public License v3.0
4 stars 3 forks source link

ZK playlists will not accept entries with whitespace in album/label fields #75

Closed bufordsharkley closed 5 years ago

bufordsharkley commented 5 years ago

Previous behavior:

1) If album/label unavailable, clicking "Add Track" with field empty would do nothing, fields would remain as-is

2) If fields entered with a single space (or other whitespace, presumably), the playlist would accept it. This was a well-known feature and utilized heavily

With changes to ZK:

1) as above 2) Playlist clears entry fields, as it would if the track was being accepted, but nothing is added to playlist, and the informations appears to be lost

I would consider this a regression, and would wish the previous behavior for supporting whitespace to return

RocketMan commented 5 years ago

Thanks for the report, Mark.

Eric, this also suggests another problem -- it would seem validations and errors from the ajax handler are not being propagated back to the user. If the handler is rejecting the insert because the fields are blank as would seem to be the case here, or if there is a failure for any reason, then the user should get a message to that effect.

eric-gilbertson commented 5 years ago

Regarding the previous behavior of spoofing the album & label fields with spaces, it seems that a better solution is simply to make them optional rather than keeping them required but accepting white space as valid input. Sampling a few playlists it appears that many DJs were not aware of this behavior as evidenced by the wide use of '.' for albums and labels.

bufordsharkley commented 5 years ago

For last week's playlist, I used ./. once, and I'm not a fan, compared to leaving a space

I agree that making them optional may be the ideal behavior, at any rate, but I would certainly push for allowing simply whitespace until this is implemented

eric-gilbertson commented 5 years ago

Given that the cost of implementation is the same for either approach, I'm inclined to go with making the album & label fields optional so that the behavior is clear to everyone. Will push a change for this (and the error display bug) soon.

RocketMan commented 5 years ago

Thanks Eric. Please make your PR against branch hotfix-2-4-0.

RocketMan commented 5 years ago

Patch landed, closing.

Thanks @eric-gilbertson for the quick turn-around. The changes have been deployed to prod. If you want to send out an announcement, please do.