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.

Comments

gábor hojtsy’s picture

Status: Active » Needs review
StatusFileSize
new682 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

StatusFileSize
new3.2 KB
new3.86 KB

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

Status: Needs work » Needs review
StatusFileSize
new1.33 KB
new3.2 KB
new3.86 KB

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.