The changes in #986992: Insane etid / {field_config_entity_type} abstraction affects Content Translation extremely, because of the big use of etid field inside it. If you are trying to use it with the latest D7 dev (where #986992: Insane etid / {field_config_entity_type} abstraction is commited), you'll get a fatal error because of deprecated function _field_sql_storage_etid() and etid field.

I'm going to work on the patch for this, and try to use entity type instead of etid.

CommentFileSizeAuthor
#1 etid.patch3.84 KBgood_man

Comments

good_man’s picture

good_man commented
StatusFileSize
new etid.patch3.84 KB

Well I was a bit confused, and thought etid == entity id, thanks to Berdir so much help in IRC to point this out. Turned out to be an easy fix.

Changes:
- Fixed the schema too.
- Replaced all etid with entity_type.
- Removed the helper function used to call the deprecated _field_sql_storage_etid()

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Status: Active » Needs review

Patch looks good, always set the status to needs review when you upload a patch, so that eventually existing tests for this project are executed.

Patch looks good, Not sure if an update function is necessary, since this is a -dev version, sun will need to decide this.

Will try the patch within this week if I find the time...

sun’s picture

sun
Primary language German
Location Karlsruhe
commented

AFAIK, @plach is using the module on production sites already, so we need a module update. However, should be simple.

@good_man: Care to attempt that module update?

plach’s picture

plach
he/him
Primary language Italian
Location Venezia
commented

I'm not using Translation on production sites, but the usage statistics tell us that 17 sites are using it right now, so perhaps we want an update function anyway. Besides this the patch looks good.

+++ translation.install	20 Dec 2010 16:17:49 -0000
@@ -42,11 +42,12 @@ function translation_schema() {
+        'default' => '',

Why are we providing a default here?

Powered by Dreditor.

pcambra’s picture

pcambra
he/him
Primary language Spanish
commented

I've tested patch with latest drupal dev release and the fatal errors went away.

+1 to module update because I had to reinstall the module completely to get this working

Thanks a ton @good_man

good_man’s picture

good_man commented

@plach: that's right, no need for default here, will work on the update today.

@pcambra: wait for the patch with the update, no need to remove it if you can wait shortly.

sun’s picture

sun
Primary language German
Location Karlsruhe
commented
Status: Needs review » Reviewed & tested by the community 
+++ translation.install	20 Dec 2010 16:17:49 -0000
@@ -42,11 +42,12 @@ function translation_schema() {
+      'entity_type' => array(
...
         'not null' => TRUE,
...
+        'default' => '',

@@ -100,7 +101,7 @@ function translation_schema() {
+    'primary key' => array('entity_type', 'entity_id', 'language'),

The default of '' is implicit for primary keys that are not null, at least on MySQL, so this is correct.

Powered by Dreditor.

sun’s picture

sun
Primary language German
Location Karlsruhe
commented
Status: Reviewed & tested by the community » Needs work

oopsie, we're still missing the module update.

plach’s picture

plach
he/him
Primary language Italian
Location Venezia
commented

Since this is rendering the module unusable and the patch was RTBC I commited it. A new dev snapshot will be available in 12 hours.

Leaving to needs work in the hope someone will provide an update function.

good_man’s picture

good_man commented

@plach, sorry for being this late, but I'm trying to catch you on IRC or any other way you prefer to discuss few things instead of opening useless issues :|

plach’s picture

plach
he/him
Primary language Italian
Location Venezia
commented

@good_man:

Send me an email through the contact form, I ain't sure I'll have time for a chat these days.

plach’s picture

plach
he/him
Primary language Italian
Location Venezia
commented
Status: Needs work » Closed (fixed)

I think we don't need an upgrade path anymore. Read #902760: Change module name so it can work with Core translation for details.