shotvibe / shotvibe-web

ShotVibe REST API webservice
1 stars 0 forks source link

For the member list of each album: keep track who of who added each user #8

Closed benny-shotvibe closed 11 years ago

benny-shotvibe commented 11 years ago

A new model needs to be added: AlbumMember (It can be called AlbumUser if you think it is more appropriate. But "member" is the terminology we are already using for album members)

It should replace the existing Album.members field (or use the Django through attribute), and additionally have fields:

The rest of the code should be adjusted to use this new model. All unit tests should (and new unit tests should be added if appropriate)

A migration should be added to convert the old Album.members into the new model (the added_by should be set to the album creator, and the date_added should be set to the date the album was created)

No changes need to be done to the REST API, for now we just need the new info stored in the database

Please develop this issue in a new git branch, and then open a pull request. (Feel free to commit early and often, and amend commits / force push if you like - the new branch will be your private branch)

prudnikov commented 11 years ago

photos and phone_auth now use south for database migrations. These commands should be executed on each dev or production server:

./manage.py syncdb
./manage.py migrate photos 0001 --fake
./manage.py migrate phone_auth 0001 --fake
./manage.py migrate photos
./manage.py migrate phone_auth

Here is more http://south.aeracode.org/wiki/ConvertingAnApp. Be careful, south will modify management commands.

prudnikov commented 11 years ago

Tests will not pass until you apply migrations.

benny-shotvibe commented 11 years ago

As we discussed, this should be the final structure of AlbumMember:

class AlbumMember(models.Model):
    user = models.ForeignKey(settings.AUTH_USER_MODEL)
    album = models.ForeignKey("Album")

    added_by_member = models.ForeignKey(settings.AUTH_USER_MODEL) # Intentionally Not Null!
    datetime_added = models.DateTimeField(auto_now_add=True)

Notes:

benny-shotvibe commented 11 years ago

Another comment:

In django, for referencing other models (or views) I personally prefer instead of using a string ("Album") to use the object directory (photos.models.Album or just Album). This makes python catch the error immediately in case of a typo or other kind of error, and also lets you use your IDE to jump directly to the definition. Of course, in cases where you must use a string (like circular references) then a string is fine

prudnikov commented 11 years ago

Regarding the string "Album" — I use a string here because the model Album is not defined at this point. In this case the solution is to use a model name as a string. https://docs.djangoproject.com/en/dev/ref/models/fields/#foreignkey

To use `photos.models.Album' you should import it first (otherwise it's undefined), but you can't import it from the same file and even before it is defined.

benny-shotvibe commented 11 years ago

yes, i see you are right, you had to use a string there because of the circular dependency between Album and AlbumMember

But once you change the format of the AlbumMember as above, then you should be able to put it below the Album model in the code

benny-shotvibe commented 11 years ago

This is completed in 8ce249cc7383892af0b12739bcf695670096ee6f