cuappdev / tempo

An open-source iOS app for music discovery and sharing with friends.
MIT License
15 stars 1 forks source link

Fix bugs that occur when two duplicate songs appear on feed from diff… #262

Closed ChenJesse closed 7 years ago

ChenJesse commented 7 years ago

…erent users

257

loganallen commented 7 years ago

If it works, looks good to me

ChenJesse commented 7 years ago

Ok I thought of a slightly better way to organize the checks, I'll get it done by tonight...I just thought we were submitting yesterday so I rushed to bug fix

say25 commented 7 years ago

SONG ID?

ChenJesse commented 7 years ago

What do you mean?

say25 commented 7 years ago

Songs have a unique Spotify given ID on them. If a user posts truly the same song it by definition has the same song id. that is probably the best way to determine duplicates

ChenJesse commented 7 years ago

Oh, the problem isn't prevent different users from posting the same song, it's dealing with how the logic determines which song is playing when two of them are identical. I do what you're talking about when I do song1.equals(other: song2), which compares IDs. But that alone is not sufficient for when two people you follow post the same song (with same songIDs, since the one further up in the tableview will always get played first)

Dennis was talking about ways to compare User objects, and I didn't see a field that is guaranteed to be unique. Username is guaranteed to be unique, the backend does a check for that whenever you're changing it, which is why I chose it. The only concern is if you change your username, but even if you change the username, the User object is still in memory so it should be fine.

dennisfedorko commented 7 years ago

I guess using username is about the same as id, so works for me

ChenJesse commented 7 years ago

Ok, fixed up the style, using id for comparing Users, and now defines Post equality as having the same equal users and songs.

Also I found a few other bugs and fixed them:

dennisfedorko commented 7 years ago

Merge it