OPENEUROPA-660: Make it impossible for a content editor to create an initial version of a content in any language other than the default language #29

sergepavle · 2018-10-05T15:09:25Z

OPENEUROPA-660

Description

Our content translation strategy requires every piece of content to be first created in the site's default language (which will in most cases be English, but there might be exceptions). Once the English version has been created any desired translations can then be added.

We should make it impossible for a content editor to create an initial version of a content in any language other than the default language.

module: "oe_multilingual"

Change log

Added:
Changed:
Deprecated:
Removed:
Fixed:
Security:

…ent only with site default language.

…date.

imanoleguskiza · 2018-10-10T10:41:38Z

tests/Behat/DrupalContext.php

+   *
+   * @Given I am visiting the :content_type_name create node page
+   */
+  public function iAmVisitingTheCreateNodePage(string $content_type_name): void {


Not sure we need this custom step. We could simply do "Given I am in the "node/add/oe_demo_translatable_page". The path is quite straightforward and easy to read, but that is my opinion. If we decide to keep the custom step I think it needs to be rewritten to something like "Given I am visiting the :content_type_name creation page"

imanoleguskiza · 2018-10-10T10:43:03Z

tests/Behat/DrupalContext.php

@@ -201,4 +202,64 @@ protected function getLanguageIdByName(string $name): string {
    throw new \InvalidArgumentException("Language '{$name}' not found.");
  }

+  /**
+   * Redirect user on Node creation page.


This should be "Redirect user to the node creation page."

imanoleguskiza · 2018-10-10T10:44:31Z

tests/Behat/DrupalContext.php

+   *
+   * @Then I should not see selectbox with label :label
+   */
+  public function iShouldNotSeeSelectboxWithLabel(string $label): void {


The name of the method (and the step) is misleading, we are not checking for selectboxes but rather for a field. We should either rename the function (and the step) or make it so that it only checks for selectboxes

imanoleguskiza · 2018-10-10T10:44:59Z

tests/Behat/DrupalContext.php

+  }
+
+  /**
+   * Check that selectbox with node language selector is hidden.


The function should allow to find any selectbox, so the comment should reflect that (and not point out to the language selector)

imanoleguskiza · 2018-10-10T10:45:43Z

tests/Behat/DrupalContext.php

+      ->getPage()
+      ->findField($label);
+    if ($element) {
+      throw new \RuntimeException("Selectbox for selecting initial language still visible.");


The error should use the provided selectbox label variable.

imanoleguskiza · 2018-10-10T10:46:40Z

tests/Behat/DrupalContext.php

+   * @param string $title
+   *   Title of node.
+   *
+   * @Then I have to be sure that :title node translation only in site default language.


A better way to word this is "@then the only available translation for :title is in the site's default language"

imanoleguskiza · 2018-10-10T10:49:38Z

tests/Behat/DrupalContext.php

+   * @Then I have to be sure that :title node translation only in site default language.
+   */
+  public function assertOnlyDefaultLanguageTranslationExist(string $title) {
+    $title_element = $this->getSession()


Instead of relying on the session for this step, wouldn't it be better to use Drupal API to gather the translations of the node we are trying to check?

nagyad · 2018-10-11T08:02:22Z

tests/features/content-default-language.feature

@@ -0,0 +1,17 @@
+@api
+Feature: Content initial language.
+  In order to be create content


In order to create content

nagyad · 2018-10-11T08:02:24Z

tests/features/content-default-language.feature

+Feature: Content initial language.
+  In order to be create content
+  As a editor
+  I want to make sure that it impossible for a content editor to create an initial version of a content in any language other


I want to make sure that it is not possible for a content editor to create a node's initial version in any language (line break here)
other than the site's default language

nagyad · 2018-10-11T08:02:27Z

tests/features/content-default-language.feature

+  I want to make sure that it impossible for a content editor to create an initial version of a content in any language other
+  than the site default language
+
+  Scenario: Content editor try to create node without possibility select initial node language


As an editor, when I create a node I can not select the initial node language

nagyad · 2018-10-11T08:31:10Z

src/MultilingualConfigOverride.php

+    foreach ($names as $config_name) {
+      // Force the default site's language as the default language and prevent
+      // the users from changing it when creating a node.
+      if (preg_match('/^language\.content_settings\.node\.(.*)$/', $config_name)) {


I think it would make this line more readable if you would use strpos()

nagyad · 2018-10-11T08:39:46Z

tests/Behat/DrupalContext.php

+    }
+
+    $node_translation_languages = $node->getTranslationLanguages();
+    if (!is_array($node_translation_languages) || count($node_translation_languages) !== 1) {


$node->getTranslationLanguages(); always returns an array.

nagyad · 2018-10-11T08:40:08Z

tests/Behat/DrupalContext.php

+   * @param string $content_type_name
+   *   Content type name.
+   *
+   * @Given I am visiting the :content_type_name creation page


@given I visit the :content_type_name creation page

nagyad · 2018-10-11T08:42:02Z

tests/Behat/DrupalContext.php

+
+    $node_translation_languages = $node->getTranslationLanguages();
+    if (!is_array($node_translation_languages) || count($node_translation_languages) !== 1) {
+      throw new \RuntimeException("We don't have the correct number of translations.");


The node should have only one translation.

src/MultilingualConfigOverride.php

sergepavle added 6 commits October 5, 2018 17:02

OPENEUROPA-660: Add override config service declaration.

6aae03c

OPENEUROPA-660: Implementation of config override class.

c6b7e75

OPENEUROPA-660: Add behat test for checking that we could create cont…

710ae18

…ent only with site default language.

OPENEUROPA-660: Add changes in behat settings.

ff93504

OPENEUROPA-660: Add additional methods in DrupalContext.

5b80979

OPENEUROPA-660: Amend text in behat tests.

763851f

sergepavle force-pushed the OPENEUROPA-660 branch from 3f6e3e9 to 3b02486 Compare October 9, 2018 14:45

OPENEUROPA-660: Fix failed behat tests with prefer-lowest composer up…

983b98c

…date.

sergepavle force-pushed the OPENEUROPA-660 branch from ccd3cd9 to 983b98c Compare October 9, 2018 14:51

sergepavle changed the title ~~Openeuropa 660~~ OPENEUROPA-660: Make it impossible for a content editor to create an initial version of a content in any language other than the default language Oct 9, 2018

imanoleguskiza suggested changes Oct 10, 2018

View reviewed changes

OPENEUROPA-660: Corrections in behat tests according to feedback.

ef5a068

sergepavle force-pushed the OPENEUROPA-660 branch from cfd7960 to ef5a068 Compare October 10, 2018 14:31

OPENEUROPA-660: Small fixes to comments.

8738311

imanoleguskiza previously approved these changes Oct 10, 2018

View reviewed changes

nagyad requested changes Oct 11, 2018

View reviewed changes

sergepavle dismissed imanoleguskiza’s stale review via f5058de October 11, 2018 09:23

OPENEUROPA-660: Rectifying english and some tiny changes.

2168681

sergepavle force-pushed the OPENEUROPA-660 branch from 8bc1577 to 2168681 Compare October 11, 2018 09:37

nagyad previously approved these changes Oct 11, 2018

View reviewed changes

imanoleguskiza previously approved these changes Oct 11, 2018

View reviewed changes

upchuk requested changes Oct 11, 2018

View reviewed changes

src/MultilingualConfigOverride.php Show resolved Hide resolved

sergepavle dismissed stale reviews from imanoleguskiza and nagyad via ec0fa14 October 11, 2018 11:20

OPENEUROPA-660: Add strict typing and return types for methods.

883083a

sergepavle force-pushed the OPENEUROPA-660 branch from ec0fa14 to 883083a Compare October 11, 2018 12:01

upchuk approved these changes Oct 11, 2018

View reviewed changes

nagyad approved these changes Oct 11, 2018

View reviewed changes

upchuk merged commit ec4fec5 into master Oct 11, 2018

upchuk deleted the OPENEUROPA-660 branch October 11, 2018 14:37

dxvargas mentioned this pull request Apr 16, 2019

OPENEUROPA-1781: Do not lock source language value to default language. #58

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

OPENEUROPA-660: Make it impossible for a content editor to create an initial version of a content in any language other than the default language #29

OPENEUROPA-660: Make it impossible for a content editor to create an initial version of a content in any language other than the default language #29

sergepavle commented Oct 5, 2018

imanoleguskiza Oct 10, 2018

sergepavle Oct 10, 2018

imanoleguskiza Oct 10, 2018

imanoleguskiza Oct 10, 2018

imanoleguskiza Oct 10, 2018

imanoleguskiza Oct 10, 2018

imanoleguskiza Oct 10, 2018

imanoleguskiza Oct 10, 2018

nagyad Oct 11, 2018

nagyad Oct 11, 2018

nagyad Oct 11, 2018

nagyad Oct 11, 2018

nagyad Oct 11, 2018

nagyad Oct 11, 2018

nagyad Oct 11, 2018

OPENEUROPA-660: Make it impossible for a content editor to create an initial version of a content in any language other than the default language #29

OPENEUROPA-660: Make it impossible for a content editor to create an initial version of a content in any language other than the default language #29

Conversation

sergepavle commented Oct 5, 2018

OPENEUROPA-660

Description

Change log

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment