Problem/Motivation

Follow-up from #1987726: Convert language content page related callback to new style controller comments #75 and #72

We did an override during a conversion so that the form would be the same, and to keep the conversion in scope and simple, an @todo was added to remove the override and update the tests that expected a particular button name.
Don't forget to enable the module Language

Proposed resolution

Do that in this issue.

Remaining tasks

User interface changes

No.

API changes

No.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

YesCT’s picture

mon_franco’s picture

Assigned: Unassigned » mon_franco
mon_franco’s picture

Status: Postponed » Needs review
FileSize
752 bytes

Patch is uploaded

mon_franco’s picture

Assigned: mon_franco » Unassigned

Status: Needs review » Needs work

The last submitted patch, 3: 2241727.patch, failed testing.

tstoeckler’s picture

Thanks for getting started on the patch!

I have one comment:

+++ b/core/modules/language/src/Form/ContentLanguageSettingsForm.php
@@ -139,7 +139,7 @@ public function buildForm(array $form, FormStateInterface $form_state) {
     // @todo Remove this override. There are tests that check for explicitly for
     //   the button label which need to be adapted for that.
     //   https://drupal.org/node/2241727

This comment should also be removed as part of the patch.

mon_franco’s picture

Assigned: Unassigned » mon_franco
mon_franco’s picture

FileSize
5.99 KB

I have looked at the next files:

  • core/modules/content_translation/src/Tests/ContentTranslationSettingsTest.php
  • core/modules/content_translation/src/Tests/ContentTranslationSyncImageTest.php
  • core/modules/language/src/Form/ContentLanguageSettingsForm.php
  • core/modules/language/src/Tests/LanguageConfigSchemaTest.php
  • core/modules/path/src/Tests/PathLanguageTest.php
  • core/modules/views/src/Tests/SearchMultilingualTest.php

And I have tried to find word "Save" and change it for "Save configuration"

mon_franco’s picture

FileSize
5.98 KB
mon_franco’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 9: 2241727.7.patch, failed testing.

alimac’s picture

Issue tags: +Amsterdam 2014
mon_franco’s picture

Status: Needs work » Needs review
FileSize
5.99 KB

Reroll. Some tests still fails, but better results now. I will continue tomorrow.

Status: Needs review » Needs work

The last submitted patch, 13: 2241727.13.patch, failed testing.

patrickd’s picture

Issue tags: -Amsterdam 2014 +Amsterdam2014

correcting amsterdam tag (No space)

mon_franco’s picture

Assigned: mon_franco » Unassigned
Status: Needs work » Needs review
FileSize
1.16 KB
5.08 KB

I have changed the file core/modules/path/src/Tests/PathLanguageTest.php because I made a mistake changing two buttons for edit nodes that should not be changed.

mon_franco’s picture

@YesCT told me next suggestion after review the last patch (#16):
"The todo is saying to remove the override. so the comment will not be needed (when the override is removed)"
So I did it.

tstoeckler’s picture

Status: Needs review » Reviewed & tested by the community

Awesomesauce!

Thanks a lot for your persistence @mon_franco!!!

mon_franco’s picture

Issue summary: View changes
mon_franco’s picture

Status: Reviewed & tested by the community » Needs work

We have found some errors by testing manually the issue

mon_franco’s picture

Status: Needs work » Needs review
FileSize
151.46 KB
143.51 KB

I have tested this issue manually and it works right as you can see on the screen shoots attached

YesCT’s picture

Status: Needs review » Reviewed & tested by the community

ok.. can't thing of anything that needs work here.

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed 08746e8 and pushed to 8.0.x. Thanks!

  • alexpott committed 08746e8 on 8.0.x
    Issue #2241727 by mon_franco | YesCT: Remove button name override and...

Status: Fixed » Closed (fixed)

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