Display id instead of error if songs already exists - #857
Conversation
EMREOYUN
left a comment
EMREOYUN
left a comment
There was a problem hiding this comment.
Your code lacks a check for already uploaded songs. It will work in that way but already uploaded songs still return "Song uploaded" with the already uploaded ID. It may be confusing. You can consider adding "Song already exists with the ID:" but still returning the ID of the already uploaded song, so users will not get confused about it.
| return "-3"; | ||
| $id = $query->fetchColumn(); | ||
| if($id != false){ | ||
| return $id; |
There was a problem hiding this comment.
Your code lacks the song existence check.
There was a problem hiding this comment.
I am not a good PHPer, but $id != false should do the trick
There was a problem hiding this comment.
I see no issues with your code.
| if($result == "-4") | ||
| echo "This URL doesn't point to a valid audio file."; | ||
| }elseif($result == "-3") | ||
| echo "This song already exists in our database."; |
There was a problem hiding this comment.
If the song exists in the database, it still returns the "song uploaded" message with the ID. It may be confusing. Consider adding "song already exists" but still returning the ID of the already uploaded song.
There was a problem hiding this comment.
I also thought about this, but the problem is about concept of this "API". It (mainlib function) is expected to return an integer where if > 0 then it is id, if <= 0 assume it failed. We should ask @Cvolton about designing this
There was a problem hiding this comment.
It still removes the capability of the error text "This song already exists in our database." Yes, your code still returns the ID of the song, very well but removing the existing error still be confusable.
I guess we need @Cvolton's opinion on that.
2 participants
Recreated pull request because of some mess with git since I accidentally did this on
masterbranch