Files: 
CommentFileSizeAuthor
#24 interdiff.txt375 bytesdawehner
#24 config_translation-2044737-24.patch8.37 KBdawehner
FAILED: [[SimpleTest]]: [MySQL] 471 pass(es), 1 fail(s), and 64 exception(s).
[ View ]
#21 config_translation-2044737-14.patch8.29 KBGábor Hojtsy
FAILED: [[SimpleTest]]: [MySQL] 471 pass(es), 1 fail(s), and 64 exception(s).
[ View ]
#16 config-translation-id.patch2.92 KBGábor Hojtsy
PASSED: [[SimpleTest]]: [MySQL] 460 pass(es).
[ View ]
#14 config_translation-2044737-14.patch11.22 KBdawehner
FAILED: [[SimpleTest]]: [MySQL] 332 pass(es), 37 fail(s), and 95 exception(s).
[ View ]
#5 config_translation-2044737-5.patch7.12 KBdawehner
FAILED: [[SimpleTest]]: [MySQL] 412 pass(es), 27 fail(s), and 0 exception(s).
[ View ]

Comments

Assigned:Unassigned» Gábor Hojtsy

Priority:Normal» Critical

Local tasks still seem to work for now, but it is highly unlikely that the module would be committed without this conversion.

Watch this issue for how we can do this: #2032309: Use local tasks derivatives to provide local tasks for views - we need to do derivative generation as well based on the mappers. It is not as simple as dropping in a few classes.

Assigned:Gábor Hojtsy» Unassigned

Ok, I think watching the views issue progress is the best we can do ATM. :)

StatusFileSize
new7.12 KB
FAILED: [[SimpleTest]]: [MySQL] 412 pass(es), 27 fail(s), and 0 exception(s).
[ View ]

There are many probles we have to figure out first, before that one can be implemented:

Nevertheless, this is a start to show how it should look like.

Status:Active» Needs review

Status:Needs review» Needs work

The last submitted patch, config_translation-2044737-5.patch, failed testing.

@dawehner: thanks for doing this! Amazing!

Just want to say, that once the menu API changes allow us to do that I will come back. This issue and probably field_ui/views will proove that the API does actually cover the usecases.

@dawehner: yay! Thanks a lot!

Issue summary:View changes

not sure why that didn't link to the issue

Issue summary:View changes

ah, because it was a change record

Issue summary:View changes

with template and missing issue

Issue summary:View changes

oops same issue

@tim.plunkett mentioned in irc that we don't want annotated plugins for local tasks. we want yaml.
and @dawehner pointed to #2050919: Replace local task plugin discovery with YamlDiscovery

Yeah this is still in flux... :/

Title:Provide annotated plugin for local tasksProvide local tasks as plugins
Status:Needs work» Needs review
StatusFileSize
new11.22 KB
FAILED: [[SimpleTest]]: [MySQL] 332 pass(es), 37 fail(s), and 95 exception(s).
[ View ]

Yeah #2076283: Allow local task plugins on paths with a dynamic value (like a node) is in!

  • This is some basic work which at least lets the tabs appear on site-information, text-formats but not on node_type edit, as this one is not converted yet to the local task plugins.
  • The active tabs on text formats are not working properly

Status:Needs review» Needs work

The last submitted patch, config_translation-2044737-14.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new2.92 KB
PASSED: [[SimpleTest]]: [MySQL] 460 pass(es).
[ View ]

Yay, thanks for your work on this.

I've extracted the id introduction only, so this can be committed ASAP. Let's see how this passes/fails.

Status:Needs review» Needs work

The last submitted patch, config-translation-id.patch, failed testing.

Every call to drupalPost should be converted to drupalPostForm() now ... so this is kind of unrelated to the patch.

One thing the patch is using is the following snippet:

+      $routes = \Drupal::service('router.route_provider')->getRoutesByPattern('/' . $config_translation_plugin['base_path']);
+      $route_names = array_keys($routes->all());
+      $route_name = array_shift($route_names);

This bits could be replaced if the config_translation plugins would also contain the route name instead of just the base path.

@dawehner: right, the drupalPost change is in #2093007: drupalPost => drupalPostForm() in tests and other core fails. We have been waiting on a core fix so we don't need to uncomment tests to make our stuff pass :) That issue should shortly come back green and get committed :)

As for route names, are all menu callbacks now converted to routes, so we can rely on that? Well, probably those that we use were converted. Mostly config entities and main site settings.

Status:Needs work» Needs review

#16: config-translation-id.patch queued for re-testing.

StatusFileSize
new8.29 KB
FAILED: [[SimpleTest]]: [MySQL] 471 pass(es), 1 fail(s), and 64 exception(s).
[ View ]

Ok, committed the ID patch with docs for the $id property at http://drupalcode.org/project/config_translation.git/commitdiff/0ec881a1... (although I used an autogenerated silly commit message, sorry :D)

Here is a quick edited version of #14 without those hunks.

Status:Needs review» Needs work

The last submitted patch, config_translation-2044737-14.patch, failed testing.

Fails with

Undefined index: > Notice LocalTaskManager.php 215 Drupal\Core\Menu\LocalTaskManager->getLocalTasksForRoute() Not sure what would that come from...

StatusFileSize
new8.37 KB
FAILED: [[SimpleTest]]: [MySQL] 471 pass(es), 1 fail(s), and 64 exception(s).
[ View ]
new375 bytes

This is caused by having not converted all the other existing local tasks for the edit tabs. Here is a todo

Status:Needs work» Needs review

Status:Needs review» Needs work

The last submitted patch, config_translation-2044737-24.patch, failed testing.

@dawehner: do you have examples of those?

#2095139: Checking for the active tab should use raw variables. is needed to convert any of the config entity local tasks, so I don't know of an example which already has an edit tab.

See #2095269: Remove add_edit_tab, core should have default tabs for some similar work, the issue @YesCT asked me to look at. Didn't know about this one.

@tim.plunkett: yeah that one was about to just remove the edit tab autogeneration. Which issue should we consolidate this then? The less issues, the less rerolls needed :) That one? This one?

That matters much less than just getting #2095271: Add default tabs for routes expected by config_translation in first, IMO. I will read through this approach again and see how to best merge them.

@tim.plunkett: yeah, sure, thanks!

Status:Needs work» Closed (duplicate)

I incorporated the pieces that are still needed from this to #2095269: Remove add_edit_tab, core should have default tabs, where some overlapping code was already there. Thanks @dawehner! Learned a lot from this patch.

Committed #2095269: Remove add_edit_tab, core should have default tabs and attributed dawehner on it :) Thanks again!

Issue summary:View changes

copy and paste mistake before.