search

shotvibe / shotvibe-web

ShotVibe REST API webservice
1 stars 0 forks source link

Add new integer column in Photo table to be used as the order of photos in an album #5

Closed benny-shotvibe closed 11 years ago

benny-shotvibe commented 11 years ago

Explanation: Right now when a batch of multiple photos are added to an album, there is a problem. The server sets the timestamp in the database for each of the photos to the same exact time. This causes the sorting of the photos to be random, instead of in the order that they were added (the order that they appeared in JSON array that the client sent)

I can think of 2 solutions:

  1. Don't set the exact same timestamp for each of the new photos. Instead: Increment the timestamp a little for each photo is slightly ahead of the previous one (even a few milliseconds should be enough, just so that they will sort properly)
  2. Add a new column to the Photo database: an integer "autoincrement" type column. In fact, the standard Django id column is a good candidate. This will make sure give automatic ordering in the database and should fix everything.

I think that solution (2) seems better (solution 1 feels like a hack), but please tell me what you think.

I think the first thing to do is to add a unit test that uploads a batch of photos, and then does a GET of the album and verifies that the order is the same. (note that with the current buggy behavior the order that is returned will be random based on the alphabetical order of the random photo ids, so the test will pass occasionally)

prudnikov commented 11 years ago

Agree, 1) is hack. 2) If you are sure that you will be able to use autoincrement id for photos (it does not scale) all the time I think ordering by ID is just enough. Also, if we add an integer column I'd name it index. Please confirm in comments what should I do. My vote is for index column.

benny-shotvibe commented 11 years ago

Ok, your idea does seem the best. Please use an index column as you suggest.

This issue needs a migration, so before starting to write the code for it, you must wait for any other active git branches that also have a migration to be merged into master, so you can branch from that, otherwise there will be merge problems with the migration code. (Please correct me if I am wrong)

prudnikov commented 11 years ago

I don't think there will be any problem. I should specify migration ID manualy. I'll see if we have anything pending for the same app with migration.

benny-shotvibe commented 11 years ago

Only other branch is issue-6-user-avatars which we will merge hopefully tomorrow so there should be no problem

benny-shotvibe commented 11 years ago

Added in ba6d494c086df638c62182dc740d40d8324146d5