Hi,

while writing integration for tmgmt i found a odd behaviour in i18n_object(). The $key can be NULL after i18n_object_key().

Here is a drush snippet to show the issue.
drush php-eval '$types = array("article", "page"); foreach($types as $type) { $o = i18n_object("node_type", $type); print_r($o); }'

CommentFileSizeAuthor
#10 i18n_object_clean-1445344-10.patch4.97 KBwebflo
#9 i18n_object_clean.patch8.85 KBjose reyero
#6 1445344-fix-with-node-test.patch3.84 KBwebflo
#4 1445344-broken-node-test.patch750 byteswebflo
#4 1445344-fix-with-node-test.patch3.84 KBwebflo
#3 1445344-fix.patch3.1 KBwebflo
#1 1445344-broken-test.patch786 byteswebflo

Comments

webflo’s picture

webflo commented
Status: Active » Needs review
StatusFileSize
new 1445344-broken-test.patch786 bytes

A test to expose the issue. This should break.

Status: Needs review » Needs work

The last submitted patch, 1445344-broken-test.patch, failed testing.

webflo’s picture

webflo commented
Status: Needs work » Needs review
StatusFileSize
new 1445344-fix.patch3.1 KB
webflo’s picture

webflo commented
StatusFileSize
new 1445344-fix-with-node-test.patch3.84 KB
new 1445344-broken-node-test.patch750 bytes

1445344-broken-node-test.patch should fail.
1445344-fix-with-node-test.patch should pass.

Status: Needs review » Needs work

The last submitted patch, 1445344-fix-with-node-test.patch, failed testing.

webflo’s picture

webflo commented
Status: Needs work » Needs review
StatusFileSize
new 1445344-fix-with-node-test.patch3.84 KB

its i18n_object_by_name().

jose reyero’s picture

jose reyero commented
Status: Needs review » Needs work

I am not that sure about this. Maybe the problem is allowing a key instead of an object in i18n_object().

What about keeping i18n_object() as the object wrapper which always takes an object parameter and adding some other i18n_object_by_key() function ? (or i18n_object_build_key())

miro_dietiker’s picture

miro_dietiker
Location Switzerland
commented 
+++ b/i18n.moduleundefined
@@ -315,12 +315,61 @@ function i18n_context_language() {
+ * @param $object_id

Object id or key?

jose reyero’s picture

jose reyero commented
Status: Needs work » Needs review
StatusFileSize
new i18n_object_clean.patch8.85 KB

This patch cleans up the i18n_object() API a little bit. I think part of the problem is using either object or object key as a parameter.

It also changes the default constructor for i18n_string_object(), not sure about this though.

webflo’s picture

webflo commented
StatusFileSize
new i18n_object_clean-1445344-10.patch4.97 KB

This looks good. I committed the whitespace for easier review and re-roll.

+++ b/i18n_object.incundefined
@@ -16,9 +17,10 @@ class i18n_object_wrapper {
+    $this->object = $object ? $object : $this->load_object($object);

Hmm i think this should be $this->load_object($key). Right?

New patch is attached.

jose reyero’s picture

jose reyero commented
Status: Needs review » Fixed
Issue tags: +Needs change record

Great, committed @webflo's version of the patch.

Also, tried the code at the top and it seems to work now.

I don't think we need anything else here, do we?
(Though I'm afraid tmgmt will need to keep depending on i18n-dev for now)

Automatically closed -- issue fixed for 2 weeks with no activity.