Files: 
CommentFileSizeAuthor
#34 2003592-lang-add-34.patch23.28 KBandypost
PASSED: [[SimpleTest]]: [MySQL] 59,148 pass(es).
[ View ]
#30 interdiff.txt8.19 KBandypost
#30 2038291-lang-add-30.patch23.28 KBandypost
PASSED: [[SimpleTest]]: [MySQL] 58,517 pass(es).
[ View ]
#29 interdiff.txt9.09 KBandypost
#29 2038291-lang-add-29.patch15.09 KBandypost
PASSED: [[SimpleTest]]: [MySQL] 58,201 pass(es).
[ View ]
#25 interdiff.txt12.08 KBandypost
#25 2038291-lang-add-25.patch15.16 KBandypost
PASSED: [[SimpleTest]]: [MySQL] 58,360 pass(es).
[ View ]
#22 interdiff.txt7.04 KBandypost
#22 2038291-lang-add-22.patch24.56 KBandypost
PASSED: [[SimpleTest]]: [MySQL] 58,102 pass(es).
[ View ]
#20 interdiff.txt953 bytesandypost
#20 2038291-lang-add-20.patch24.49 KBandypost
FAILED: [[SimpleTest]]: [MySQL] 57,835 pass(es), 34 fail(s), and 0 exception(s).
[ View ]
#18 interdiff.txt3.36 KBandypost
#18 2038291-lang-add-18.patch24.52 KBandypost
FAILED: [[SimpleTest]]: [MySQL] 58,079 pass(es), 36 fail(s), and 2 exception(s).
[ View ]
#16 2038291-lang-add-16.patch24.28 KBandypost
FAILED: [[SimpleTest]]: [MySQL] 57,553 pass(es), 543 fail(s), and 146 exception(s).
[ View ]
#9 language-edit-form-controller-2003592-9.patch9.2 KBwebflo
FAILED: [[SimpleTest]]: [MySQL] 57,137 pass(es), 45 fail(s), and 20 exception(s).
[ View ]
#3 language-wscci-conversion-2003592-3.patch7.76 KBmhagedon
FAILED: [[SimpleTest]]: [MySQL] 56,926 pass(es), 45 fail(s), and 7 exception(s).
[ View ]

Comments

Getting started on this shortly...

StatusFileSize
new7.76 KB
FAILED: [[SimpleTest]]: [MySQL] 56,926 pass(es), 45 fail(s), and 7 exception(s).
[ View ]

Here's a partial patch. It'll fail testing, but on top of that the functional tests are blocked by #1952394: Add configuration translation user interface module in core. Next step is probably to figure out how best to populate the Language object in buildForm. Not marking "needs review" since it will fail.

Status:Active» Needs review
Issue tags:+D8MI, +language-base

Status:Needs review» Needs work

The last submitted patch, language-wscci-conversion-2003592-3.patch, failed testing.

Assigned:mhagedon» Pancho

Rather this one partly depends on #2005778: Convert language_admin_overview_form to a Controller.
Also, this one and #2038291: Convert language_admin_add_form to a Controller are heavily interdepending, so we need to refactor the logic from composition to inheritance. Well, we don't necessarily have to - both have their merits even in OOP -, but it would make the code cleaner plus improve the experience with JS being disabled.

So I started a patch that should be CNR by tomorrow, if I find time finishing it. Hope it's okay with @mhagedon if I reassign this to me until Sunday or so.

#2005778: Convert language_admin_overview_form to a Controller landed, including LanguageAccessController.

Status:Needs work» Needs review
StatusFileSize
new9.2 KB
FAILED: [[SimpleTest]]: [MySQL] 57,137 pass(es), 45 fail(s), and 20 exception(s).
[ View ]

This patch adds a new LanguageFormController. Maybe we should postpone #2038291: Convert language_admin_add_form to a Controller?

Status:Needs review» Needs work

The last submitted patch, language-edit-form-controller-2003592-9.patch, failed testing.

Too bad, this seriously has been double work. :(
It's not been without reason that I said I'm assigning to this until Sunday.

Issue tags:+API change, +API clean-up

Add API change tags.

Ouch, finishing my patch is overdue. I'm sorry, it will be my next task for today.

Postponed #2038291: Convert language_admin_add_form to a Controller on this one here.

@Pancho, @webflo, still working on this one?

Title:Convert language_admin_edit_form to a ControllerConvert language_admin_add_form and language_admin_edit_form to a Controllers

Closed as duplicate #2038291: Convert language_admin_add_form to a Controller

Both forms have common part so needs base class like image styles uses.
Also I'm not sure that there's api change because _language_admin_common_controls() is not API function so should be moved to the base class.

Suppose the names should be:
Form/LanguageAddForm + Form/LanguageEditForm inherited from abstract Form/LanguageFormBase with corresponding inhected services by implementing EntityControllerInterface

And require #2031277-11: Implement checkCreateAccess on LanguageAccessController

Status:Needs work» Needs review
StatusFileSize
new24.28 KB
FAILED: [[SimpleTest]]: [MySQL] 57,553 pass(es), 543 fail(s), and 146 exception(s).
[ View ]

Suppose approach should be different, initial patch

Status:Needs review» Needs work

The last submitted patch, 2038291-lang-add-16.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new24.52 KB
FAILED: [[SimpleTest]]: [MySQL] 58,079 pass(es), 36 fail(s), and 2 exception(s).
[ View ]
new3.36 KB

Another attempt, in chrome html5 validation still disallow submit predefined language

Status:Needs review» Needs work

The last submitted patch, 2038291-lang-add-18.patch, failed testing.

Status:Needs work» Needs review
Issue tags:+Needs manual testing
StatusFileSize
new24.49 KB
FAILED: [[SimpleTest]]: [MySQL] 57,835 pass(es), 34 fail(s), and 0 exception(s).
[ View ]
new953 bytes

Fix some test failures, still can't fix #states for predefined language in chrome

Status:Needs review» Needs work

The last submitted patch, 2038291-lang-add-20.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new24.56 KB
PASSED: [[SimpleTest]]: [MySQL] 58,102 pass(es).
[ View ]
new7.04 KB

Removed standard actions
Renamed validation
a bit of clean-up

Issue tags:+JavaScript

Tagging because hidden #state does not allows to submit Add form by mouse, but "space-button" works.
Also this works before conversion

PS: chrome 30.0.1599.10 dev

  1. +++ b/core/modules/language/lib/Drupal/language/Form/LanguageAddForm.php
    @@ -0,0 +1,141 @@
    +    parent::commonForm($form['custom_language']);
    +++ b/core/modules/language/lib/Drupal/language/Form/LanguageEditForm.php
    @@ -0,0 +1,60 @@
    +    parent::commonForm($form);

    This is not a proper use of parent::, it should be something like

    $form = $this->addCommonFormElements($form, $form_state);

  2. +++ b/core/modules/language/lib/Drupal/language/Form/LanguageEditForm.php
    @@ -0,0 +1,60 @@
    +    $actions['submit'] = array(
    +      '#type' => 'submit',
    +      '#value' => $this->translator->translate('Save language'),
    +      '#validate' => array(array($this, 'validateCommon')),
    +      '#submit' => array(array($this, 'submitForm')),
    +    );

    This doesn't make much sense to me.

  3. +++ b/core/modules/language/lib/Drupal/language/Form/LanguageFormBase.php
    @@ -0,0 +1,114 @@
    +   */
    +  protected $translator;

    This should have used t() until FormBase was in.

  4. +++ b/core/modules/language/lib/Drupal/language/Form/LanguageFormBase.php
    @@ -0,0 +1,114 @@
    +  public function validateCommon(array $form, array &$form_state) {

    Why isn't this just validateForm()?

StatusFileSize
new15.16 KB
PASSED: [[SimpleTest]]: [MySQL] 58,360 pass(es).
[ View ]
new12.08 KB

Fixed translator

Status:Needs review» Needs work

t() could be just replaced by $this->t() in this patch.

+++ b/core/modules/language/lib/Drupal/language/Form/LanguageAddForm.php
@@ -0,0 +1,144 @@
+    // @todo Remove this when https://drupal.org/node/1981644 is in.
+    drupal_set_title(t('Add language'));

Let's set the #title on the render array.

There's a conceptual question here - old code and currently used functions operates on Language object not the language entity.
So should we build the forms on top of the language entity... for me it seems weird (the difference in label vs name properties)

+++ b/core/modules/language/language.routing.yml
@@ -19,6 +19,20 @@ language_negotiation_selected:
+    _entity_form: 'language_entity.add'
...
+    _entity_create_access: 'language_entity'
...
+  pattern: '/admin/config/regional/language/edit/{language_entity}'
...
+    _entity_form: 'language_entity.edit'
...
+    _entity_access: 'language_entity.update'
+++ b/core/modules/language/lib/Drupal/language/Entity/Language.php
@@ -24,7 +24,11 @@
+ *       "add" = "Drupal\language\Form\LanguageAddForm",
+ *       "edit" = "Drupal\language\Form\LanguageEditForm"

Language entity faked with 'name' form element so language_save() could continue to work and form structure is not changed

Yeah the add form and edit form edit/add entities, they should be entity form controllers as for any other config entity.

Status:Needs work» Needs review
StatusFileSize
new15.09 KB
PASSED: [[SimpleTest]]: [MySQL] 58,201 pass(es).
[ View ]
new9.09 KB

Re-roll for $this->t() and #title

Related #1946426: Convert all of confirm_form() in language.admin.inc to the new form interface uses entity too

StatusFileSize
new23.28 KB
PASSED: [[SimpleTest]]: [MySQL] 58,517 pass(es).
[ View ]
new8.19 KB

Missed the removal of old code

Thanks for your work on this issue! Please see #1971384-43: [META] Convert page callbacks to controllers for an update on the routing system conversion process.

Status:Needs review» Reviewed & tested by the community

This one looks great and passes tests! Marking RTBC!

Issue tags:+sprint

Agreed, let's get this in!

StatusFileSize
new23.28 KB
PASSED: [[SimpleTest]]: [MySQL] 59,148 pass(es).
[ View ]

re-roll

Status:Reviewed & tested by the community» Fixed

Committed and pushed to 8.x. Thanks!

Issue tags:-sprint

Yay, thanks!

Thanks all!

Yay, thanks all!

Status:Fixed» Closed (fixed)
Issue tags:+sprint

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

Issue tags:-sprint

Thanks all!

Issue summary:View changes

Updated issue summary.