google-code-export / django-modeltranslation

Automatically exported from code.google.com/p/django-modeltranslation
BSD 3-Clause "New" or "Revised" License
0 stars 0 forks source link

Add translation fields to base class when multi-table inheritance is used #50

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
What steps will reproduce the problem?
1. Make a base class A with the field title, do not mark as abstract
2. Make a class B which is a subclass of A and adds it's own fields
3. Register the title-field in class B for translation 
4. Call B.objects.all()

What is the expected output? What do you see instead?
It should return all instances of B, instead it generates incorrect SQL:
'SELECT "A"."id", "A"."title", "B"."A_ptr_id", "B"."title_is", "B"."title_en", 
"B"."someotherfield" FROM "B" INNER JOIN "A" ON ("B"."a_ptr_id" = "A"."id")'
As a result I get an error:
Caught DatabaseError while rendering: column B.title_is does not exist...

What version of the product are you using? On what operating system?
Django 1.3 django-modeltranslation-trunk

Please provide any additional information below.
It would be great if I could just register the base class for translation, but 
if I try to do that I get a NotRegistered error when making queries on the 
inherited class.

Original issue reported on code.google.com by hr.bja...@gmail.com on 30 Sep 2010 at 5:28

GoogleCodeExporter commented 9 years ago
Inherited classes are supported by modeltranslation and in fact we make heavy 
use of them.

As you haven't included your model definition and your modeltranslation related 
settings, i can only guess that you have registered B.title for translation 
after your initial run of syncdb. In this case you have to add title_is (and 
title_en) manually to your database. Or in case the tables don't hold any data, 
you can also just drop them and run syncdb again.

Original comment by eschler on 1 Oct 2010 at 8:41

GoogleCodeExporter commented 9 years ago
Hi, thanks for the quick reply.
Sorry, I had registered the base class and not the sub class.

What is wrong with it though (conceptually) is that the translated fields 
title_is and title_en are created in the database table for the subclass but 
not in the base class table. This means that if I want to make a function in 
the base class that queries those fields, I can't and have to make the function 
in all the subclasses slightly different.

This bit me now when I had a field called tags in the base class, with field 
type TagField (provided by the django-tagging application, basically just a 
CharField).

I wanted to make a function to retrieve the original string as input by the 
user, since the TagField returns an ordered, comma-less version of the users 
input.

So I'd like to do it like this:
from django.utils.translation import get_language
def get_tags(self):
    retval = u""
    try:
        from django.db import connection, transaction
        cursor = connection.cursor()
        cursor.execute("SELECT tags_%s FROM A WHERE id = %d" % (get_language(), self.pk))
        row = cursor.fetchone()
        retval = row[0]
    except Exception, inst:
        pass
    finally:
        return retval.rstrip(u" ,")

But that won't work because the translated field is in the sub class even if 
the original field is in the base class.
As a result I have to make slightly different versions of this function in all 
of the sub classes.

Is this something you're willing to change? As it is now it seems conceptually 
wrong. 

Original comment by hr.bja...@gmail.com on 1 Oct 2010 at 11:57

GoogleCodeExporter commented 9 years ago
By the way, I found a workaround for this particular problem:
def get_efnisord(self):
    retval = u""
    try:
        from django.db import connection, transaction
        cursor = connection.cursor()
        cursor.execute("SELECT tags_%s FROM %s WHERE A_ptr_id = %d" % (get_language(), self._meta.db_table, self.pk))
            row = cursor.fetchone()
            retval = row[0]
        except Exception, inst:
            pass
        finally:
            return retval.rstrip(u" ,")

But this does feel a bit hackish and I still think that conceptually the 
translated fields should be stored in the table of the model they belong to.

Original comment by hr.bja...@gmail.com on 1 Oct 2010 at 12:17

GoogleCodeExporter commented 9 years ago
Thanks for the clarification. I get your point now and it's a valid one. Most 
of the inherited models i had used with modeltranslation were abstract ones so 
i've never run into this problem.

The problem you describe occurs with multi-table inheritance and has indeed to 
be fixed. In the essence modeltranslation has to distinct between abstract and 
multi-table inheritance.

Original comment by eschler on 1 Oct 2010 at 12:39

GoogleCodeExporter commented 9 years ago

Original comment by eschler on 10 Apr 2011 at 9:51

GoogleCodeExporter commented 9 years ago
Hi,
I came up with a patch you'll find on github: 
https://github.com/hyperweek/django-modeltranslation/commit/08b560321cb0896a5b1c
dbc5ed5238cb7c6d1984
Feel free to test/merge it.

Original comment by sebastie...@gmail.com on 1 Jun 2011 at 2:09

GoogleCodeExporter commented 9 years ago
Very nice. The patch looks good to me. Tests run fine. I'd be glad to include 
it in the 0.3 release. Thanks alot for your help!

Original comment by eschler on 1 Jun 2011 at 3:05

GoogleCodeExporter commented 9 years ago
Resolved in r111.

Original comment by eschler on 1 Jun 2011 at 3:27

GoogleCodeExporter commented 9 years ago

Original comment by eschler on 1 Jun 2011 at 3:27

GoogleCodeExporter commented 9 years ago
@sebastian
I found a problem with the admin integration. When you have localized fields in 
both, the parent class and the subclass and both are registered for 
translation, only the subclass is handled properly. The original field of the 
parent class isn't hidden and the modeltranslation specific css classes aren't 
applied, so jquery-ui-tabs don't work either. Do you see a good way of keeping 
track of the related fieldnames in translation options? We can't rely on the 
KeyError in this case, as both models are registered.

Original comment by eschler on 16 Jun 2011 at 1:56

GoogleCodeExporter commented 9 years ago
Reopened due to problems mentioned in comment 10.

Original comment by eschler on 7 Dec 2011 at 12:16

GoogleCodeExporter commented 9 years ago

Original comment by eschler on 10 Jul 2012 at 9:06

GoogleCodeExporter commented 9 years ago
Project moved to Github. All issues have been migrated preserving their id. All 
remaining open issues on GoogleCode are closed. New project url: 
https://github.com/deschler/django-modeltranslation

Original comment by eschler on 22 Oct 2012 at 8:22