Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 11 commits into from
Oct 11, 2018
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions behat.yml.dist
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ default:
contexts:
- Drupal\DrupalExtension\Context\MinkContext
- Drupal\DrupalExtension\Context\DrupalContext
- Drupal\DrupalExtension\Context\MessageContext
- Drupal\Tests\oe_multilingual\Behat\DrupalContext
- Drupal\Tests\oe_multilingual\Behat\MinkContext
extensions:
Expand All @@ -18,6 +19,8 @@ default:
api_driver: "drupal"
drupal:
drupal_root: "build"
selectors: &drupal-selectors
success_message_selector: ".messages--status"
region_map:
"language switcher": "#block-oe-multilingual-language-switcher"
"language dialog": "#block-oe-multilingual-language-switcher"
Expand Down
5 changes: 3 additions & 2 deletions composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,9 @@
"drush/drush": "~9.0@stable",
"openeuropa/code-review": "^1.0.0-alpha4",
"openeuropa/drupal-core-require-dev": "^8.6",
"openeuropa/task-runner": "~1.0",
"phpunit/phpunit": "~6.0"
"openeuropa/task-runner": "~1.0.0-beta2",
"phpunit/phpunit": "~6.0",
"symfony/browser-kit": "~3.0||~4.0"
},
"scripts": {
"post-install-cmd": "./vendor/bin/run drupal:site-setup",
Expand Down
4 changes: 4 additions & 0 deletions oe_multilingual.services.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,7 @@ services:
oe_multilingual.helper:
class: Drupal\oe_multilingual\MultilingualHelper
arguments: ['@entity.repository', '@current_route_match', '@language_manager']
oe_multilingual.content_language_settings:
class: Drupal\oe_multilingual\MultilingualConfigOverride
tags:
- {name: config.factory.override, priority: 5}
52 changes: 52 additions & 0 deletions src/MultilingualConfigOverride.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
<?php

sergepavle marked this conversation as resolved.
Show resolved Hide resolved
namespace Drupal\oe_multilingual;

use Drupal\Core\Cache\CacheableMetadata;
use Drupal\Core\Config\ConfigFactoryOverrideInterface;
use Drupal\Core\Config\StorageInterface;

/**
* Override some config values for customizing default behavior.
*/
class MultilingualConfigOverride implements ConfigFactoryOverrideInterface {

/**
* {@inheritdoc}
*/
public function loadOverrides($names) {
$overrides = [];
foreach ($names as $config_name) {
if (preg_match('/^language\.content_settings\.node\.(.*)$/', $config_name)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

$overrides[$config_name] = [
'default_langcode' => 'site_default',
'language_alterable' => FALSE,
];
}
}

return $overrides;
}

/**
* {@inheritdoc}
*/
public function createConfigObject($name, $collection = StorageInterface::DEFAULT_COLLECTION) {
return NULL;
}

/**
* {@inheritdoc}
*/
public function getCacheSuffix() {
return 'oe_multilingual.language_configs_override';
}

/**
* {@inheritdoc}
*/
public function getCacheableMetadata($name) {
return new CacheableMetadata();
}

}
61 changes: 61 additions & 0 deletions tests/Behat/DrupalContext.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
use Behat\Behat\Hook\Scope\AfterScenarioScope;
use Behat\Behat\Hook\Scope\BeforeScenarioScope;
use Behat\Gherkin\Node\TableNode;
use Behat\Mink\Element\NodeElement;
use Drupal\Core\Entity\ContentEntityInterface;
use Drupal\DrupalExtension\Context\RawDrupalContext;
use Drupal\field\Entity\FieldConfig;
Expand Down Expand Up @@ -201,4 +202,64 @@ protected function getLanguageIdByName(string $name): string {
throw new \InvalidArgumentException("Language '{$name}' not found.");
}

/**
* Redirect user on Node creation page.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

*
* @param string $content_type_name
* Content type name.
*
* @Given I am visiting the :content_type_name create node page
*/
public function iAmVisitingTheCreateNodePage(string $content_type_name): void {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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"

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

$node_bundle = $this->getEntityTypeByLabel($content_type_name);
$this->visitPath('node/add/' . $node_bundle);
}

/**
* Check that selectbox with node language selector is hidden.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

*
* @param string $label
* Label of selectbox.
*
* @Then I should not see selectbox with label :label
*/
public function iShouldNotSeeSelectboxWithLabel(string $label): void {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

$element = $this->getSession()
->getPage()
->findField($label);
if ($element) {
throw new \RuntimeException("Selectbox for selecting initial language still visible.");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The error should use the provided selectbox label variable.

}
}

/**
* Check that we have correct language for initial translation.
*
* @param string $title
* Title of node.
*
* @Then I have to be sure that :title node translation only in site default language.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

*/
public function assertOnlyDefaultLanguageTranslationExist(string $title) {
$title_element = $this->getSession()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

->getPage()
->findAll('named', ['link', $title]);

if (count($title_element) !== 1) {
throw new \RuntimeException("We have not correct number of translations.");
}
if ($title_element[0] instanceof NodeElement) {
$translation_language = $title_element[0]
->getParent()
->getParent()
->findAll('named', ['content', \Drupal::languageManager()->getDefaultLanguage()->getName()]);
if (!count($translation_language)) {
throw new \RuntimeException("Original translation of node not equal to default site language.");
}
}
else {
throw new \RuntimeException("Not found translation of node.");
}
}

}
19 changes: 19 additions & 0 deletions tests/features/content-default-language.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
@api
Feature: Content initial language.
In order to be create content
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In order to 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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

than the site default language

Scenario: Content editor try to create node without possibility select initial node language
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Given I am logged in as a user with the "create oe_demo_translatable_page content,translate oe_demo_translatable_page node" permission
And I am visiting the "Demo translatable page" create node page
Then I should not see selectbox with label "Language"

When I fill in "Title" with "Title Default value"
And I fill in "Body" with "Body Default value"
And I press "Save"
Then I should see the success message "Demo translatable page Title Default value has been created."

When I click "Translate"
Then I have to be sure that "Title Default value" node translation only in site default language.