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-480: Provide a language selection page #5

Merged
merged 18 commits into from
May 29, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
ed46723
OPENEUROPA-480: WIP.
richardcanoe May 22, 2018
26d9978
OPENEUROPA-480: Fix configuration import.
ademarco May 22, 2018
a440d50
OPENEUROPA-480: Refactor hook_install().
ademarco May 22, 2018
7b2efeb
OPENEUROPA-480: Add behat test.
ademarco May 22, 2018
73eb7a8
OPENEUROPA-480: Add theme branch to composer.
richardcanoe May 23, 2018
1148e4f
OPENEUROPA-480: Make sure language selection page is available when v…
ademarco May 23, 2018
2522db4
OPENEUROPA-480: Pull request fixes.
richardcanoe May 25, 2018
723dac9
OPENEUROPA-480: Removed dependency module install.
richardcanoe May 25, 2018
bcf857b
Merge remote-tracking branch 'origin/master' into OPENEUROPA-480
netlooker May 25, 2018
483e82a
OPENEUROPA-480: Fixing typo to move tests on Drone further.
netlooker May 28, 2018
8dd5578
OPENEUROPA-480: Make sure language selection page negotiation is adde…
ademarco May 28, 2018
f079c89
Merge branch 'OPENEUROPA-480' of github.com:openeuropa/oe_multilingua…
ademarco May 28, 2018
cd7662e
OPENEUROPA-480: Add comment about optional module's configuration.
ademarco May 29, 2018
cb5f3df
Merge branch 'master' into OPENEUROPA-480
ademarco May 29, 2018
52c18fb
OPENEUROPA-480: Set oe_theme version to ^0.2.
ademarco May 29, 2018
46bb0fe
OPENEUROPA-480: Remove devel from composer requires.
ademarco May 29, 2018
da72ab3
OPENEUROPA-480: Add README.md to OpenEuropa Multilingual Selection Pa…
ademarco May 29, 2018
4465652
OPENEUROPA-480: Add strict_types and remove obsolete isActive flag.
ademarco May 29, 2018
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: 1 addition & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,6 @@ Then:
$ docker-compose exec -u web web composer install
$ docker-compose exec -u node node npm install
$ docker-compose exec -u node node npm run build
$ docker-compose exec -u web web ./vendor/bin/run drupal:site-setup
$ docker-compose exec -u web web ./vendor/bin/run drupal:site-install
```

Expand Down Expand Up @@ -170,7 +169,7 @@ This is due to the following [Drupal Console issue][11].
## Demo module

The OpenEuropa Multilingual module ships with a demo module which provides all the necessary configuration and code needed
to showcase the modules's most important features.
to showcase the modules' most important features.

The demo module includes a translatable content type with automatic URL path generation.

Expand Down
1 change: 1 addition & 0 deletions behat.yml.dist
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ default:
- Drupal\DrupalExtension\Context\MinkContext
- Drupal\DrupalExtension\Context\DrupalContext
- Drupal\Tests\oe_multilingual\Behat\DrupalContext
- Drupal\Tests\oe_multilingual\Behat\MinkContext
extensions:
Behat\MinkExtension:
goutte: ~
Expand Down
8 changes: 5 additions & 3 deletions composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,11 @@
"minimum-stability": "dev",
"prefer-stable": true,
"require": {
"php": "^7.1",
"drupal/administration_language_negotiation": "1.4",
"drupal/core": "~8.6",
"drupal/pathauto": "1.2",
"php": "^7.1"
"drupal/language_selection_page": "^2.2",
"drupal/pathauto": "^1.2"
},
"require-dev": {
"cweagans/composer-patches": "~1.0",
Expand All @@ -18,10 +19,11 @@
"drupal/console": "^1.6",
"drupal/config_devel": "~1.2",
"drush/drush": "^9",
"drupal/devel": "^1.2",
"drupal/drupal-extension": "^4.0.0@alpha",
"openeuropa/code-review": "^0.2",
"openeuropa/task-runner": "^0.5",
"openeuropa/oe_theme": "dev-master",
"openeuropa/oe_theme": "^0.2",
"webflo/drupal-core-require-dev": "8.6.x"
},
"scripts": {
Expand Down
7 changes: 7 additions & 0 deletions modules/oe_multilingual_selection_page/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
# OpenEuropa Multilingual Selection Page

The OpenEuropa Multilingual Selection Page module will present users with a language selection page, as soon as their
visit the site.

**Disclaimer**: when installed this module will update existing configuration by enabling the language selection page
negotiation method, make sure you review these changes before deploying your configuration.
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
name: 'OpenEuropa Multilingual Selection Page'
type: module
description: 'Generates a Language Selection Page used on first access to a multilingual site.'
core: 8.x
package: 'OpenEuropa'

dependencies:
- drupal:language_selection_page
- oe_multilingual:oe_multilingual
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
<?php

/**
* @file
* Install, update and uninstall functions for the module.
*/

declare(strict_types = 1);

use Drupal\language_selection_page\Plugin\LanguageNegotiation\LanguageNegotiationLanguageSelectionPage;

/**
* Implements hook_install().
*/
function oe_multilingual_selection_page_install(): void {
// Configure selection page language negotiation method.
\Drupal::configFactory()
->getEditable('language_selection_page.negotiation')
->set('path', '/select-language')
->set('type', 'standalone')
->set('ignore_neutral', FALSE)
->set('blacklisted_paths', [
'/admin',
'/user',
'/admin/*',
'/admin*',
'/node/add/*',
'/node/*/edit',
])->save();
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 setting configuration in an install hook, the recommended way to provide default configuration is by exporting it into a YAML file and placing it in the config/install subfolder.

Copy link
Member

Choose a reason for hiding this comment

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

This configuration is not provided by the oe_multilingual_selection_page so it cannot be stored in its config/install module, doing that will result in an error at installation time.

Copy link
Member

Choose a reason for hiding this comment

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

config/optional then?

Copy link
Member

Choose a reason for hiding this comment

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

It would have the same effect, the configuration is in the config/install of the module this module depends on, so that will be installed first, causing the same error.

Copy link
Member

@pfrenssen pfrenssen May 29, 2018

Choose a reason for hiding this comment

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

OK let's go with the install hook, but there is a risk involved with this. If the site builder enables this module, then decides not to use it and uninstalls it again, the change to the language negotiation will not be undone. I would not suggest to revert to the previous language negotiation strategy either, since there might be a very long time between the enabling and uninstallation of the module, and the site may have become dependent on the language negotiation provided by this module.

What I think we need is a disclaimer / opt-in checkbox when enabling the module, and also documentation in the README that explains to site builders that enabling this module will set the administrative language to English.

Is there anything we can revert safely on hook_uninstall()? Maybe we can remove the language_selection_page.negotiation part from config and the LanguageNegotiationLanguageSelectionPage::METHOD_ID from the language negotiation?

I don't think we can touch the other things on uninstall though :-/

Providing a disclaimer and opt-in checkbox on installation is out of scope for this ticket, let's do this in a followup after discussion. We can put a @todo here though linking to the followup, and make a note in the README.


/** @var \Drupal\oe_multilingual\LanguageNegotiationSetterInterface $setter */
$setter = \Drupal::service('oe_multilingual.language_negotiation_setter');

// Add language selection page negotiation method.
// Since this is an optional module setting configuration in its
// hook_install() might cause unexpected behaviors.
// We are discussing implications in the following ticket:
// https://webgate.ec.europa.eu/CITnet/jira/browse/OPENEUROPA-600
$setter->addInterfaceSettings([
LanguageNegotiationLanguageSelectionPage::METHOD_ID => -18,
]);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
<?php

/**
* @file
* Contains hooks to manipulate language links for selection page.
*/

Copy link
Member

Choose a reason for hiding this comment

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

Let's declare strict typing.

Copy link
Member

Choose a reason for hiding this comment

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

This is not yet done.

declare(strict_types = 1);

/**
* Implements hook_page_preprocess().
*/
function oe_multilingual_selection_page_preprocess_language_selection_page_content(array &$variables): void {

// Generate an array suitable for use in the ecl-language-list component.
foreach ($variables['language_links']['#items'] as $key => $value) {
$language_code = $key;
$url = $value['#url']->toString();

$variables['languages'][] = [
'href' => $url,
'hreflang' => $language_code,
'label' => $value['#title'],
'lang' => $language_code,
];
}
}
36 changes: 19 additions & 17 deletions oe_multilingual.install
Original file line number Diff line number Diff line change
Expand Up @@ -36,29 +36,31 @@ function oe_multilingual_install() {
'/node/*/translations',
])->save();

// Make sure that English language prefix is set to "en".
\Drupal::configFactory()
->getEditable('language.negotiation')
->set('url.prefixes.en', 'en')
->save();
Copy link
Member

Choose a reason for hiding this comment

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

This seems strange, we set the language negotiation for English, but leave the rest untouched? It seems clear to me that the goal of OE Multilingual is to provide the full language negotiation tactic. Just overriding English won't be enough since there are no guarantees that the other settings are matching the defaults from Drupal core.

In other words, this will work fine only if the site builder hasn't been messing with the language negotiation before enabling this module.

Copy link
Member

Choose a reason for hiding this comment

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

The other language settings are not coming from core but from our module's ./config/install so we "control them" so to say. The English language is the only one coming from core, that's why it's special handed.

About the site builder messing with configuration: we won't have this case since the OpenEuropa components are meant to be enabled at the beginning of the development process, the site configuration will be then exported in the site's config/sync and committed to the site repo.

I do agree, however, that this might generate confusion, we should either document this or have a talk with the team about this, there is also a ticket about configuration management, we could deal with this in that ticket's scope.


/** @var \Drupal\oe_multilingual\LanguageNegotiationSetterInterface $setter */
$setter = \Drupal::service('oe_multilingual.language_negotiation_setter');

// Set default language negotiation methods.
$setter->enableNegotiationMethods([
LanguageInterface::TYPE_INTERFACE,
LanguageInterface::TYPE_CONTENT,
]);

// For interface negotiation make sure administrative pages are in English.
$interface_settings = [
$setter->setInterfaceSettings([
LanguageNegotiationAdministrationLanguage::METHOD_ID => -20,
LanguageNegotiationUrl::METHOD_ID => -19,
LanguageNegotiationSelected::METHOD_ID => 20,
];
]);

// For content negotiation make sure that content respects URL language.
$content_settings = [
$setter->setContentSettings([
LanguageNegotiationUrl::METHOD_ID => -19,
LanguageNegotiationSelected::METHOD_ID => 20,
];

// Set default language negotiation methods with related weights.
\Drupal::configFactory()
->getEditable('language.types')
->set('configurable', [
LanguageInterface::TYPE_INTERFACE,
LanguageInterface::TYPE_CONTENT,
])
->set('negotiation.' . LanguageInterface::TYPE_INTERFACE . '.enabled', $interface_settings)
->set('negotiation.' . LanguageInterface::TYPE_INTERFACE . '.method_weights', $interface_settings)
->set('negotiation.' . LanguageInterface::TYPE_CONTENT . '.enabled', $content_settings)
->set('negotiation.' . LanguageInterface::TYPE_CONTENT . '.method_weights', $content_settings)
->save();
]);
}
3 changes: 3 additions & 0 deletions oe_multilingual.services.yml
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
services:
oe_multilingual.language_negotiation_setter:
class: Drupal\oe_multilingual\LanguageNegotiationSetter
arguments: ['@config.factory']
oe_multilingual.helper:
class: Drupal\oe_multilingual\MultilingualHelper
arguments: ['@entity.repository', '@current_route_match', '@language_manager']
1 change: 1 addition & 0 deletions runner.yml.dist
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ drupal:
password: ""
post_install:
- "./vendor/bin/drush en config_devel -y"
- "./vendor/bin/drush en devel -y"
- "./vendor/bin/drush en oe_multilingual -y"
- "./vendor/bin/drush en oe_multilingual_demo -y"
- "./vendor/bin/drush pmu big_pipe -y"
Expand Down
94 changes: 94 additions & 0 deletions src/LanguageNegotiationSetter.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
<?php

declare(strict_types = 1);

namespace Drupal\oe_multilingual;

use Drupal\Core\Config\ConfigFactoryInterface;
use Drupal\Core\Language\LanguageInterface;

/**
* Helper service for language negotiation method configuration.
*/
class LanguageNegotiationSetter implements LanguageNegotiationSetterInterface {

/**
* Configuration name.
*/
const CONFIG_NAME = 'language.types';

/**
* Drupal\Core\Config\ConfigFactoryInterface definition.
*
* @var \Drupal\Core\Config\ConfigFactoryInterface
*/
protected $configFactory;

/**
* Constructs a new LanguageNegotiationSetter object.
*/
public function __construct(ConfigFactoryInterface $config_factory) {
$this->configFactory = $config_factory;
}

/**
* {@inheritdoc}
*/
public function enableNegotiationMethods(array $methods): void {
$this->configFactory
->getEditable(self::CONFIG_NAME)
->set('configurable', $methods)
->save();
}

/**
* {@inheritdoc}
*/
public function setInterfaceSettings(array $settings): void {
$this->configFactory
->getEditable(self::CONFIG_NAME)
->set('negotiation.' . LanguageInterface::TYPE_INTERFACE . '.enabled', $settings)
->set('negotiation.' . LanguageInterface::TYPE_INTERFACE . '.method_weights', $settings)
->save();
}

/**
* {@inheritdoc}
*/
public function addInterfaceSettings(array $settings): void {
$current = $this->configFactory
->get(self::CONFIG_NAME)
->get('negotiation.' . LanguageInterface::TYPE_INTERFACE . '.enabled');

$settings = array_merge($current, $settings);
asort($settings);

$this->setInterfaceSettings($settings);
}

/**
* {@inheritdoc}
*/
public function setContentSettings(array $settings): void {
$this->configFactory
->getEditable(self::CONFIG_NAME)
->set('negotiation.' . LanguageInterface::TYPE_CONTENT . '.enabled', $settings)
->set('negotiation.' . LanguageInterface::TYPE_CONTENT . '.method_weights', $settings)
->save();
}

/**
* {@inheritdoc}
*/
public function addContentSettings(array $settings): void {
$current = $this->configFactory
->get(self::CONFIG_NAME)
->get('negotiation.' . LanguageInterface::TYPE_CONTENT . '.enabled');

$settings = array_merge($current, $settings);
asort($settings);

$this->setContentSettings($settings);
}

}
82 changes: 82 additions & 0 deletions src/LanguageNegotiationSetterInterface.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
<?php

declare(strict_types = 1);

namespace Drupal\oe_multilingual;

/**
* Helper service for language negotiation method configuration.
*/
interface LanguageNegotiationSetterInterface {

/**
* Enable given language negotiation methods.
*
* @param array $methods
* Array of language negotiation method names.
*/
public function enableNegotiationMethods(array $methods): void;

/**
* Set interface language negotiation settings.
*
* Usage:
*
* \Drupal::service('oe_multilingual.language_negotiation_setter')
* ->setInterfaceSettings([
* LanguageNegotiationUrl::METHOD_ID => -19,
* LanguageNegotiationSelected::METHOD_ID => 20,
* ]);
*
* @param array $settings
* Array of language negotiation method names with their weights.
*/
public function setInterfaceSettings(array $settings): void;

/**
* Add given settings to current interface language negotiation.
*
* Usage:
*
* \Drupal::service('oe_multilingual.language_negotiation_setter')
* ->addInterfaceSettings([
* LanguageNegotiationUrl::METHOD_ID => -19,
* ]);
*
* @param array $settings
* Array of language negotiation method names with their weights.
*/
public function addInterfaceSettings(array $settings): void;

/**
* Set content language negotiation settings.
*
* Usage:
*
* \Drupal::service('oe_multilingual.language_negotiation_setter')
* ->setContentSettings([
* LanguageNegotiationUrl::METHOD_ID => -19,
* LanguageNegotiationSelected::METHOD_ID => 20,
* ]);
*
* @param array $settings
* Array of language negotiation method names with their weights.
*/
public function setContentSettings(array $settings): void;

/**
* Add given settings to current content language negotiation.
*
* Usage:
*
* \Drupal::service('oe_multilingual.language_negotiation_setter')
* ->addContentSettings([
* LanguageNegotiationUrl::METHOD_ID => -19,
* ]);
*
* @param array $settings
* Array of language negotiation method names with their weights.
*/
public function addContentSettings(array $settings): void;

}
Loading