Multilingual poll choices

As I have expressed my opinion to Roberto at his blocks issue:

Shared data! We discussed to a lot of length, that a node based solution allows a superior feature set, compared to anything out of nodes. But there are quite a few situations, when shared data is a requirement. A common solution for sharing data among nodes should solve the problem for polls, events and simpler use cases, where only a few properties of a node are translated. Unfortunately I have see no solution for node instance data sharing yet in Drupal core/contrib, so it seems we have no prior art to look at.

Sharing data between nodes would be the ultimate solution here. Life would be a lot easier for us if everything would be a standalone CCK field (although it would be prohibited by performance requirements probably).

Moving to Drupal 7 queue. We don't have a separate code repository now, so no reason to have this separated as if someone is working on it.

This should wait for the outcome of: #367595: Translatable fields, also changing priority temporarily for i18n sprint.

with help of t function?
stupid and hard coded way but I couldnt get over any other way!

Cleaning-up the "language system" issue queue as per http://cyrve.com/criticals.

Poll module should leverage the field API to fix this.

Effectively means better_poll_field module in core, replacing current poll module. Love, D8.

Poll is no longer in core

Poll is now an entity. My aim is to include support for the question and choice fields so each poll may be fully multilingual.

Started testing this. Works pretty well with some fixes.

One issue is the choice ID, which is auto-incremented. It matches if you add translations of existing choices, but adding a new one to the original and then a translation will result in different ID's, don't know yet how to address that.

The problem that ID is trying to solve is that you can re-order choices.

Rerolled, fixed the test, still needs test.

Added tests, so the interdiff is like a test only.

The only change is the addition of content_translation_ui_skip = TRUE, in PollChoice.

1. +++ b/src/Tests/PollTestBase.php
   @@ -36,7 +36,13 @@ abstract class PollTestBase extends WebTestBase {
       */
   -  public static $modules = array('poll', 'node', 'block');
   +  public static $modules = [
   +    'poll',
   +    'node',
   +    'block',
   +    'language',
   +    'content_translation',
   +  ];
    
      /**
   

   This means every test now enables those modules. I'd prefer to only do that in the test that needs them.

   You need an admin user with that permission, which is why you did that. In tmgmt, we solved that problem by making the list of permissions a property on the base class and subclasses can override it ($this->adminPermissions)

2. +++ b/src/Tests/PollTestBase.php
   @@ -48,8 +54,19 @@ abstract class PollTestBase extends WebTestBase {
   +      'translate any entity',
   

   there should be a poll specific permission too I think.

3. +++ b/src/Tests/PollVoteMultilingualTest.php
   @@ -0,0 +1,118 @@
   +/**
   + * @file
   + * Definition of Drupal\poll\Tests\PollVoteMultilingualTest.
   + */
   +
   +namespace Drupal\poll\Tests;
   +use Drupal\Core\Session\AccountInterface;
   +use Drupal\language\Entity\ConfigurableLanguage;
   

   Contains \Drupal..., not Definition of Drupal..

   missing empty line after namespace.

4. +++ b/src/Tests/PollVoteMultilingualTest.php
   @@ -0,0 +1,118 @@
   +    // Allow anonymous users to vote on polls.
   +    user_role_change_permissions(AccountInterface::ANONYMOUS_ROLE, array(
   +      // 'vote on polls' => TRUE,
   +      'cancel own vote' => TRUE,
   +      'access polls' => TRUE,
   +    ));
   

   why do we need this here? is there a specific bug we're testing against?

5. +++ b/src/Tests/PollVoteMultilingualTest.php
   @@ -0,0 +1,118 @@
   +
   +    // Translate a poll.
   +    $this->drupalGet('poll/1/translations');
   

   Use $this->poll, don't hardcode the id.

6. +++ b/src/Tests/PollVoteMultilingualTest.php
   @@ -0,0 +1,118 @@
   +    // Record a vote for the first choice.
   +    $edit = array(
   +      'choice' => '1',
   

   pick a different than the first choice, as we'll delete that, see below.

7. +++ b/src/Tests/PollVoteMultilingualTest.php
   @@ -0,0 +1,118 @@
   +    // Allow anonymous to vote.
   +    $this->drupalLogin($this->admin_user);
   +    $this->drupalGet('ca/poll/' . $this->poll->id() . '/edit');
   +    $edit = array(
   +      'anonymous_vote_allow[value]' => TRUE,
   

   This seems to duplicate setUp() ? same question as there, why do we need this?

8. +++ b/src/Tests/PollVoteMultilingualTest.php
   @@ -0,0 +1,118 @@
   +    // Vote as anonymous user.
   +    $this->drupalLogout();
   +    $edit = array(
   +      'choice' => '1',
   +    );
   

   More hardcoded ID's. I think a helper method to get the choice based on the choice label similar to drupalgetNodeByTitle() would be useful so we don't need that. There's something like that already somewhere but it's afaik also based on the field delta.

9. +++ b/src/Tests/PollVoteMultilingualTest.php
   @@ -0,0 +1,118 @@
   +    $this->drupalGet('poll/' . $this->poll->id());
   +    $elements = $this->xpath('//input[@value="Cancel vote"]');
   +    $this->assertTrue(isset($elements[0]), "'Cancel vote' button appears.");
   +    $this->assertText('Total votes:  2', 'Vote count updated correctly.');
   

   So far so good, but we're missing the actually interesting things:

   Edit the poll, remove the first option, add a new one. Go to the translation, translate the label there.

   Go back to the poll, vote on the new option on the translation.

   Make sure that the existing votes are correct and so is the new one. Check the actual specific votes, not just the total. See the delete test, I did that there too. Also make sure that the deleted choice is gone. All that in both languages.

Yes, those tests where not testing anything, as testing the new things the translation of the choices was not working, now with the help of @Berdir it works. And it's tested well.
Also I found a bug and I opened an issue #2648802: Deleting a choice should delete the votes over it in the database
Also I'll open a issue in Core to find a solution to not need to create PollChoiceTranslationMetadataWrapper and PollChoiceTranslationHandler when you don't need the fields added by ContentTranslationHandler.

I opened the issue in Core #2648822: Make metadata optional in ContentTranslationHandler

+++ b/src/PollChoiceTranslationMetadataWrapper.php
@@ -0,0 +1,112 @@
+  /**
+   * Updates a field value, only if the field is translatable.
+   *
+   * @param string $field_name
+   *   The name of the field.
+   * @param mixed $value
+   *   The field value to be set.
+   */
+  protected function setFieldOnlyIfTranslatable($field_name, $value) {}

is this needed? why not @inheritdoc?

Nitpick. $this->WebUserPermissions should be webUserPermissions.

Done.

Awesome. Committed.