Make feeds/feed items multilingual

What do you imagine would be the requirements here?

I think just being able to specify a language for a feed and that language is then copied to the feed item would be a considerable improvement to what we have now.

Definitely :) I don't think we want to have multilingual properties on feeds or feed items, at least I cannot think of a need for that. Those syncing site content via feeds would use node entities as their target anyway (not with the core modules).

Not sure if the tests are going to work and this will need test coverage but manual tests worked fine. Allowed me to set a language and the feed items take over that language.

Failed with this:

exception 'PDOException' with message 'SQLSTATE[23000]: Integrity constraint violation: 1062 Duplicate entry 'John' for key 'name'' in /var/lib/drupaltestbot/sites/default/files/checkout/core/lib/Drupal/Core/Database/Statement.php:58
Stack trace:
#0 /var/lib/drupaltestbot/sites/default/files/checkout/core/lib/Drupal/Core/Database/Statement.php(58): PDOStatement->execute(Array)
#1 /var/lib/drupaltestbot/sites/default/files/checkout/core/lib/Drupal/Core/Database/Connection.php(523): Drupal\Core\Database\Statement->execute(Array, Array)
#2 /var/lib/drupaltestbot/sites/default/files/checkout/core/lib/Drupal/Core/Database/Driver/mysql/Insert.php(34): Drupal\Core\Database\Connection->query('INSERT INTO {vi...', Array, Array)
#3 /var/lib/drupaltestbot/sites/default/files/checkout/core/modules/views/lib/Drupal/views/Tests/ViewUnitTestBase.php(43): Drupal\Core\Database\Driver\mysql\Insert->execute()
#4 /var/lib/drupaltestbot/sites/default/files/checkout/core/modules/simpletest/lib/Drupal/simpletest/TestBase.php(714): Drupal\views\Tests\ViewUnitTestBase->setUp()
#5 /var/lib/drupaltestbot/sites/default/files/checkout/core/scripts/run-tests.sh(404): Drupal\simpletest\TestBase->run()
#6 /var/lib/drupaltestbot/sites/default/files/checkout/core/scripts/run-tests.sh(22): simpletest_script_run_one_test('678', 'Drupal\views\Te...')
#7 {main}

Next exception 'Drupal\Core\Database\IntegrityConstraintViolationException' with message 'SQLSTATE[23000]: Integrity constraint violation: 1062 Duplicate entry 'John' for key 'name': INSERT INTO {views_test_data} (name, age, job, created) VALUES (:db_insert_placeholder_0, :db_insert_placeholder_1, :db_insert_placeholder_2, :db_insert_placeholder_3), (:db_insert_placeholder_4, :db_insert_placeholder_5, :db_insert_placeholder_6, :db_insert_placeholder_7), (:db_insert_placeholder_8, :db_insert_placeholder_9, :db_insert_placeholder_10, :db_insert_placeholder_11), (:db_insert_placeholder_12, :db_insert_placeholder_13, :db_insert_placeholder_14, :db_insert_placeholder_15), (:db_insert_placeholder_16, :db_insert_placeholder_17, :db_insert_placeholder_18, :db_insert_placeholder_19); Array
(
    [:db_insert_placeholder_0] => John
    [:db_insert_placeholder_1] => 25
    [:db_insert_placeholder_2] => Singer
    [:db_insert_placeholder_3] => 946684800
    [:db_insert_placeholder_4] => George
    [:db_insert_placeholder_5] => 27
    [:db_insert_placeholder_6] => Singer
    [:db_insert_placeholder_7] => 946771200
    [:db_insert_placeholder_8] => Ringo
    [:db_insert_placeholder_9] => 28
    [:db_insert_placeholder_10] => Drummer
    [:db_insert_placeholder_11] => 946708230
    [:db_insert_placeholder_12] => Paul
    [:db_insert_placeholder_13] => 26
    [:db_insert_placeholder_14] => Songwriter
    [:db_insert_placeholder_15] => 946706400
    [:db_insert_placeholder_16] => Meredith
    [:db_insert_placeholder_17] => 30
    [:db_insert_placeholder_18] => Speaker
    [:db_insert_placeholder_19] => 946708210
)
' in /var/lib/drupaltestbot/sites/default/files/checkout/core/lib/Drupal/Core/Database/Connection.php:551
Stack trace:
#0 /var/lib/drupaltestbot/sites/default/files/checkout/core/lib/Drupal/Core/Database/Driver/mysql/Insert.php(34): Drupal\Core\Database\Connection->query('INSERT INTO {vi...', Array, Array)
#1 /var/lib/drupaltestbot/sites/default/files/checkout/core/modules/views/lib/Drupal/views/Tests/ViewUnitTestBase.php(43): Drupal\Core\Database\Driver\mysql\Insert->execute()
#2 /var/lib/drupaltestbot/sites/default/files/checkout/core/modules/simpletest/lib/Drupal/simpletest/TestBase.php(714): Drupal\views\Tests\ViewUnitTestBase->setUp()
#3 /var/lib/drupaltestbot/sites/default/files/checkout/core/scripts/run-tests.sh(404): Drupal\simpletest\TestBase->run()
#4 /var/lib/drupaltestbot/sites/default/files/checkout/core/scripts/run-tests.sh(22): simpletest_script_run_one_test('678', 'Drupal\views\Te...')
#5 {main}FATAL Drupal\views\Tests\Handler\FieldXssTest: test runner returned a non-zero error code (1).

#4: feed-langcode-1905870-4.patch queued for re-testing.

#4: feed-langcode-1905870-4.patch queued for re-testing.

And now OpenID tests failed. So looks like unrelated flukes. Retesting again.

Hm, that's a strange fail:

No CKEditor plugins found besides the built-in ones.	Other	CKEditorPluginManagerTest.php	70	Drupal\ckeditor\Tests\CKEditorPluginManagerTest->testEnabledPlugins()

Possibly a new random one?

Anyway, it doesn't matter if this doesn't get green, needs work is the correct state as it needs tests.

Yeah, there are always different fails :) I agree it needs tests anyway. It would in all likeliness pass alone if not for the flukes on testbot.

Ok, added basic test coverage.

Gabor, do you think we need something else here? There isn't much to test, the language is currently not visible nor does it provide any additional features. This will only start to get interesting once the views integration makes it in, which should now be updated to include the language fields/filters.

Yeah, this will get interesting with views. I don't think there were any added features to aggregator, so there is still no global admin page for items is there? That could get a language filter, if there is one such screen now. Code review notes:

+++ b/core/modules/aggregator/aggregator.installundefined
@@ -293,3 +307,31 @@ function aggregrator_update_8000() {
+  db_update('aggregator_item')
+    ->fields(array('langcode' => LANGUAGE_DEFAULT))
+    ->execute();
+}
\ No newline at end of file

Missing newline at end of file.

+++ b/core/modules/aggregator/lib/Drupal/aggregator/Tests/FeedLanguageTest.phpundefined
@@ -0,0 +1,79 @@
+    $this->createSampleNodes();

Create sample nodes?!

+++ b/core/modules/aggregator/lib/Drupal/aggregator/Tests/FeedLanguageTest.phpundefined
@@ -0,0 +1,79 @@
+        $this->assertEqual($item->language()->langcode, $feed->language()->langcode);
+      }
+    }
+  }
+}
\ No newline at end of file

Missing newline at the end of file here too.

Fixed the file endings, add an assertions that there actually were feed items loaded (because it would pass if there would be none otherwise) and moved and commented the sample nodes method call.

Looks very nice. Thanks for the updates :)

Looks great. Committed/pushed to 8.x.

Could use a change notice.

Added http://drupal.org/node/1918090

+
+/**
+ * Adds the langcode field to {aggregator_item} and {aggregator_feed}.
+ */
+function aggregator_update_8001() {
+  // Add the field.
+  db_add_field('aggregator_feed', 'langcode', array(
+    'description' => 'The {language}.langcode of this feed.',
+    'type' => 'varchar',
+    'length' => 12,
+    'not null' => TRUE,
+    'default' => '',
+  ));
+  db_add_field('aggregator_item', 'langcode', array(
+    'description' => 'The {language}.langcode of this feed item.',
+    'type' => 'varchar',
+    'length' => 12,
+    'not null' => TRUE,
+    'default' => '',
+  ));
+  // Set langcode to LANGUAGE_DEFAULT.
+  db_update('aggregator_feed')
+    ->fields(array('langcode' => LANGUAGE_DEFAULT))
+    ->execute();
+  db_update('aggregator_item')
+    ->fields(array('langcode' => LANGUAGE_DEFAULT))
+    ->execute();
+}

We have the 'initial' key for use in the specification. Less code, and I'd say more clarity.

Oh, didn't know or forgot about that.

Initial does exactly the same thing internally, so this looks good to me.

lol i didn't know about that either. when did that get in :) ?

Yay two line patches. :)

Committed and pushed to 8.x. Thanks!