Problem/Motivation

The following code reads settings from where it should be but then saves to somewhere else in NegotiationSessionForm.php:

  /**
   * {@inheritdoc}
   */
  public function buildForm(array $form, FormStateInterface $form_state) {
    $config = $this->config('language.negotiation');
    $form['language_negotiation_session_param'] = array(
      '#title' => $this->t('Request/session parameter'),
      '#type' => 'textfield',
      '#default_value' => $config->get('session.parameter'),
      '#description' => $this->t('Name of the request/session parameter used to determine the desired language.'),
    );

    $form_state->setRedirect('language.negotiation');

    return parent::buildForm($form, $form_state);
  }

  /**
   * {@inheritdoc}
   */
  public function submitForm(array &$form, FormStateInterface $form_state) {
    $this->config('language.settings')
      ->set('session.parameter', $form_state->getValue('language_negotiation_session_param'))
      ->save();

    parent::submitForm($form, $form_state);
  }

Proposed resolution

Save to the right place.
Add tests.

Remaining tasks

Add tests.

User interface changes

None.

API changes

None.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Gábor Hojtsy’s picture

Status: Active » Needs review
FileSize
682 bytes

Fix is easy. Needs tests.

Wim Leers’s picture

Hah!

penyaskito’s picture

Added tests for session negotiation. Tests this issue as a side effect.

penyaskito’s picture

Forgot to add ::runTest() to the patch.

The last submitted patch, 3: 2359879-session-settings-3.test-only.patch, failed testing.

The last submitted patch, 3: 2359879-session-settings-3.patch, failed testing.

The last submitted patch, 4: 2359879-session-settings-4.test-only.patch, failed testing.

Gábor Hojtsy’s picture

Status: Needs review » Needs work

Yay, amazing. Thanks for these tests.

+++ b/core/modules/language/src/Tests/LanguageUILanguageNegotiationTest.php
@@ -317,9 +318,37 @@ function testUILanguageNegotiation() {
+        'expected_method_id' => LanguageNegotiatorInterface::METHOD_ID,
+        'http_header' => $http_header_browser_fallback,
+        'message' => 'SESSION > DEFAULT: language given, UI language is determined by session language preference',
...
+        'expected_method_id' => LanguageNegotiationSession::METHOD_ID,
+        'http_header' => $http_header_browser_fallback,
+        'message' => 'SESSION > DEFAULT: no language given, the UI language is default',

The assert messages are backwards? Need to be flipped.

penyaskito’s picture

Oops, sorry.

(╯°□°)╯︵ ┻━┻

Fixed.

Gábor Hojtsy’s picture

Status: Needs review » Reviewed & tested by the community

Looks great, thanks a lot!

The last submitted patch, 9: 2359879-session-settings-9.only-test.patch, failed testing.

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed 455a6a6 and pushed to 8.0.x. Thanks!

  • alexpott committed 455a6a6 on 8.0.x
    Issue #2359879 by penyaskito, Gábor Hojtsy: Fixed Session negotiation...
Gábor Hojtsy’s picture

Issue tags: -sprint

Yay, thanks all!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.