views:

2887

answers:

7

I have two models, Room and Image. Image is a generic model that can tack onto any other model. I want to give users a form to upload an image when they post information about a room. I've written code that works, but I'm afraid I've done it the hard way, and specifically in a way that violates DRY.

Was hoping someone who's a little more familiar with django forms could point out where I've gone wrong.

Update:

I've tried to clarify why I chose this design in comments to the current answers. To summarize:

I didn't simply put an ImageField on the Room model because I wanted more than one image associated with the Room model. I chose a generic Image model because I wanted to add images to several different models. The alternatives I considered were were multiple foreign keys on a single Image class, which seemed messy, or multiple Image classes, which I thought would clutter my schema. I didn't make this clear in my first post, so sorry about that.

Seeing as none of the answers so far has addressed how to make this a little more DRY I did come up with my own solution which was to add the upload path as a class attribute on the image model and reference that every time it's needed.

# Models
class Image(models.Model):
    content_type = models.ForeignKey(ContentType)
    object_id = models.PositiveIntegerField()
    content_object = generic.GenericForeignKey('content_type', 'object_id')
    image = models.ImageField(_('Image'),
                                height_field='',
                                width_field='',
                                upload_to='uploads/images',
                                max_length=200)
class Room(models.Model):
    name = models.CharField(max_length=50)
    image_set = generic.GenericRelation('Image') 

# The form
class AddRoomForm(forms.ModelForm):
    image_1 = forms.ImageField()

    class Meta:
        model = Room

# The view
def handle_uploaded_file(f):

    # DRY violation, I've already specified the upload path in the image model
    upload_suffix = join('uploads/images', f.name)
    upload_path = join(settings.MEDIA_ROOT, upload_suffix)
    destination = open(upload_path, 'wb+')
    for chunk in f.chunks():
        destination.write(chunk)
    destination.close()
    return upload_suffix

def add_room(request, apartment_id, form_class=AddRoomForm, template='apartments/add_room.html'):
    apartment  = Apartment.objects.get(id=apartment_id)

    if request.method == 'POST':
        form = form_class(request.POST, request.FILES)
        if form.is_valid():
            room = form.save()
            image_1 = form.cleaned_data['image_1']

            # Instead of writing a special function to handle the image, 
            # shouldn't I just be able to pass it straight into Image.objects.create
            # ...but it doesn't seem to work for some reason, wrong syntax perhaps?

            upload_path = handle_uploaded_file(image_1)
            image = Image.objects.create(content_object=room, image=upload_path)
            return HttpResponseRedirect(room.get_absolute_url())
    else:
        form = form_class()
    context = {'form': form, }
    return direct_to_template(request, template, extra_context=context)
+1  A: 

Why don't you do just use a ImageField? I don't see the need for the "Image" class.

# model
class Room(models.Model):
    name = models.CharField(max_length=50)
    image = models.ImageField(upload_to="uploads/images/")


# form
from django import forms

class UploadFileForm(forms.Form):
    name = forms.CharField(max_length=50)
    image  = forms.FileField()

Take a look at Basic file uploads and How do I use image and file fields?

DZPM
+1: Use the features of the tool
S.Lott
I used a separate image class because I want an arbitrary number of images on the Room model. For that I think I need to have a separate model class with a foreign key to Room at least. It's generic because I want an arbitrary number of images on more than one Model.
Prairiedogg
+2  A: 

You don't have to use the Image class. As DZPM suggested, convert the image field to an ImageField. You also need to make some changes to the view.

Instead of using an upload handler, you can create a Image object with the uploaded data and attach the Image object to the Room object.

To save the Image object you need to do something like this in the view:

from django.core.files.base import ContentFile

if request.FILES.has_key('image_1'):
    image_obj = Image()
    image_obj.file.save(request.FILES['image_1'].name,\
                        ContentFile(request.FILES['image_1'].read()))
    image_obj.save()
    room_obj.image_set.create(image_obj)
    room_obj.save()

Also, I think instead of the GenericRelation, you should use a ManyToManyField, in which case the syntax for adding an Image to a Room will change slightly.

Baishampayan Ghose
I don't need one image to be related to multiple Rooms, so I think m2m is overkill, a simple foreign key should do...right guys? Guys?
Prairiedogg
Yeah, use a ForeignKey then. Update the models in the question and I will tell you how to write the view.
Baishampayan Ghose
I appreciate it, but tying the Image class to a single Model isn't what I want here. The question was how to make this DRY or use convention to make it cleaner. I resolved the DRY issue (see update) and also moved the Image creation logic from the view to the form.save() method. Works for me.
Prairiedogg
A: 

What about using two forms on the page: one for the room and one for the image?

You'll just have to make the generic foreign key fields of the image form not required, and fill in their values in the view after saving the room.

akaihola
A: 

Django does support your use case at least up to a point:

  • formsets display repeated forms
  • model formsets handle repeated model forms
  • inline formsets bind model formsets to related objects of an instance
  • generic inline formsets do the same for generic relations

Generic inline formsets were introduced in changeset [8279]. See the changes to unit tests to see how they are used.

With generic inline formsets you'll also be able to display multiple already saved images for existing rooms in your form.

Inline formsets seem to expect an existing parent instance in the instance= argument. The admin interface does let you fill in inlines before saving the parent instance, so there must be a way to achieve that. I've just never tried that myself.

akaihola
At a certain point with the problem I was looking at generic inline formsets. They do seem to be exactly what I want to use, but at the time I just couldn't make it work. Thanks for the tip.
Prairiedogg
A: 

I found this page look for a solution to this same problem.

Here is my info -- hopefully helps some.

MODELS: Image, Review, Manufacturer, Profile

I want Review, Manufacturer, Profile to have a relationship to the Image model. But you have to beable to have multiple Images per object. (Ie, One Review can have 5 images a different Review can have 3, etc)

Originally I did a

images = ManyToManyField(Image)

in each of the other models. This works fine, but sucks for admin (combo select box). This may be a solution for you though. I dont like it for what I'm trying to do.

The other thing I'm working on now is having multiple foreign keys.

class Image(models.Model):
    description = models.TextField(blank=True)
    image = models.ImageField(upload_to="media/")
    user_profile = models.ForeignKey(UserProfile)
    mfgr = models.ForeignKey(Manufacturer)
    review = models.ForeignKey(Review)

but like you said. This is pretty sloppy looking and I just dont like it.

One other thing I just found but don't have my brain completely wrapped around (and not sure how transparent it is after implementation is Generic Relationships (or Generic Foreign Keys), which may be a solution. Well once I comprehend it all. Need more caffeine.

http://www.djangoproject.com/documentation/models/generic_relations/

Let me know if you get this sorted out or any of this helps. Thanks!

Let me know if this helps or you have a different solutions.

A: 

Ok I figured it out with some more reading... I feel like you want to do exactly what I have done so here it is.

I'll be using GenericForeignKeys for this.

First the imports for models.py

from django.contrib.contenttypes.models import ContentType
from django.contrib.contenttypes import generic

Now add the following to your Image Model

class Image(models.Model):
    content_type = models.ForeignKey(ContentType)
    object_id = models.PositiveIntegerField()
    content_object = generic.GenericForeignKey()

This lets this model be just that, a Generic Foreign Key for any number of models. Then add the following to all the models you want to have related images

images = generic.GenericRelation(Image)

Now in admin.py you need to add the following things.

from django.contrib.contenttypes.generic import GenericTabularInline

class ImageInline(GenericTabularInline):
    model = Image
    extra = 3
    ct_field_name = 'content_type'
    id_field_name = 'object_id'

And then include it in a admin declaration

class ReviewAdmin(admin.ModelAdmin):
    inlines = [ImageInline]

And thats it. Its working great over here. Hope this helps man! .adam.

If you read my original question, this is exactly what I've done from a schema perspective (go back and look at my Image model). My use case was for something outside the admin, but if it was in the admin, yeah, this is more or less what I would do.
Prairiedogg
A: 

Use two forms, one for the room and one for the image:

class Image(models.Model)

content_type = models.ForeignKey(ContentType)
object_id = models.PositiveIntegerField()
content_object = generic.GenericForeignKey('content_type', 'object_id')
image = models.ImageField(upload_to='')

class UploadImage(forms.ModelForm):

class Meta:
    model = Image
    fields = ('image')

class Room(models.Model):

name = models.CharField(max_length=50)
images = models.ManyToManyField(Image) 

class RoomForm(forms.ModelForm):

class Meta:
    model = Room

in the views

if request.method == "POST":

    ##2 form, una per l'annuncio ed una per la fotografia
    form = RoomForm(request.POST)
    image_form = UploadImage(request.POST, request.FILES)
    #my_logger.debug('form.is_valid() : ' + str(form.is_valid()))
    if form.is_valid() and image_form.is_valid():
        ##save room
        room = room.save()

        ##save image
        image = image_form.save()

        ##ManyToMany
        room.images = [image]
        room.save()
Alain Maringoni