Updated: Comment #0

Problem/Motivation

Follow-up for #1855402-110: Add generic weighting (tabledrag) support for config entities (DraggableListController)
The entities are serialized with all injections

<berdir> andypost: I think what it does right now is confusing yes. and scary
<berdir> andypost: try this debug(serialize($form), $form_id); in a hook_form_alter() for a draggable form
<berdir> andypost: there's *everything* in there ;)
<berdir> request, full container, module lists, database connection, ...

Proposed resolution

TBD

Remaining tasks

TBD

User interface changes

-

API changes

-

#2004282: Injected dependencies and serialization hell

Files: 
CommentFileSizeAuthor
#15 draggable-2074875-15.patch3.16 KBmtift
PASSED: [[SimpleTest]]: [MySQL] 59,448 pass(es).
[ View ]
#7 draggable-2074875-7.patch3.16 KBtim.plunkett
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch draggable-2074875-7.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#7 interdiff.txt910 bytestim.plunkett
#4 draggable-2074875-4.patch3.14 KBtim.plunkett
FAILED: [[SimpleTest]]: [MySQL] 58,494 pass(es), 18 fail(s), and 27 exception(s).
[ View ]

Comments

Component:entity system» configuration entity system
Issue tags:+Configuration system, +Configurables

Sure, doing $this->storage->loadMultiple($form_state['values'][$this->entitiesKey]) in submit is fine. I actually had that.
The only reason I moved it into $this->entities was because of locale_form_language_admin_overview_form_alter(), which assumes it has access to the full list of entities (in $form['languages']['#languages']).

Taxonomy should hide weight header's column when there's only one vocabulary

The title is a bit misleading, this isn't about DX in any way. The serialize stuff actually comes from the form class itself and I hope that will improve with the now RTBC' serialization issue.

This just feels weird to me, given how these form classes work, to store the entities on the class and re-use them. At least validate() runs on a cached form, so I'm really unsure on what list of entities they will actually run, the one that got serialized before?

Title:Provide better DX for DraggableListControllerReload entities in DraggableListController::submitForm()
Status:Active» Needs review
StatusFileSize
new3.14 KB
FAILED: [[SimpleTest]]: [MySQL] 58,494 pass(es), 18 fail(s), and 27 exception(s).
[ View ]

Here's one approach.

Status:Needs review» Needs work

The last submitted patch, draggable-2074875-4.patch, failed testing.

Assigned:Unassigned» tim.plunkett

Missed the failure message.

Status:Needs work» Needs review
StatusFileSize
new910 bytes
new3.16 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch draggable-2074875-7.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Oh, that was silly. Missed an array_keys()

Assigned:tim.plunkett» Unassigned

Great

Status:Needs review» Reviewed & tested by the community

This really awesome!

+++ b/core/modules/locale/locale.module
@@ -724,7 +724,7 @@ function locale_library_info_alter(&$libraries, $module) {
+  $languages = $form_state['build_info']['callback_object']->load();

this needs comment in upcoming change notice, because it's only example of alter in core to access entities within list controller

Status:Reviewed & tested by the community» Needs review

Suppose @berdir's approval is needed here.
I'd prefer to store a list of IDs in form_state or actually in state() service for cache, because load() should happen on cached list

Assigned:Unassigned» andypost
Status:Needs review» Needs work

Taking over, talked in IRC

timplunkett, I know that there's 3rd way to solve this by taking serialization - so alter should run on loaded entities within form builder, the validation and submit should have the same list of loaded entities

I don't really understand #10/#11 yet but the previous patch looks fine to me.

The entity ID's are already available as element_children($form[$entity_type]), are they not? At least that's what the submit method assumes as well (the only children in that form element being entity id's)

Status:Needs work» Needs review
Issue tags:-Configuration system, -Configurables

#7: draggable-2074875-7.patch queued for re-testing.

Status:Needs review» Needs work
Issue tags:+Configuration system, +Configurables

The last submitted patch, draggable-2074875-7.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new3.16 KB
PASSED: [[SimpleTest]]: [MySQL] 59,448 pass(es).
[ View ]

straight reroll

+++ b/core/modules/locale/locale.module
@@ -694,7 +694,7 @@ function locale_library_info_alter(&$libraries, $module) {
-  $languages = $form['languages']['#languages'];
+  $languages = $form_state['build_info']['callback_object']->load();

this should not happen, so we need to store entities, just get rid of serialization

Assigned:andypost» Unassigned
Issue summary:View changes

ni idea how to prevent serialization