Follow-up to #2285413: [Meta] Standardize entity route names

Problem/Motivation

Original meta issue needs this follow-up to clean-up field_ui provided routes

Proposed resolution

Extract changes from #2281645: Make entity annotations use link templates instead of route names

Remaining tasks

file a patch

User interface changes

None.

API changes

field_ui routes will have different names.

Beta phase evaluation

Reference: https://www.drupal.org/core/beta-changes
Issue category Task, because the field_ui routes simply got forgotten, when many of the other routes got renamed
Issue priority Normal, because this will "just" allow auto-generation of this limited amount of routes.
Disruption Tiny disruption, due to renamed routes. The amount of code which links to some field UI routes is probably quite limited.
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

m1r1k’s picture

Assigned: Unassigned » m1r1k
Issue tags: +Amsterdam2014
andypost’s picture

Status: Active » Needs review
FileSize
7.29 KB

fix routes for entity view|form displays

andypost’s picture

Assigned: m1r1k » andypost
FileSize
948 bytes
8.21 KB

Makes sure that entity has bundle, suppose the scope allows

PS: working to fix routes for fields

Crell’s picture

This looks fine to me, but it's unclear from the comment in #3 if you are still working on something or if you're clarifying what it's for. :-) Should I RTBC it or is there another patch coming?

andypost’s picture

Patch is not ready, there's routes for field and field config, also this affects content translation which routes needs some clean-up... see comment in #2281645-81: Make entity annotations use link templates instead of route names
The CT routes are tricky but affected by field_ui so probably better to fit them here too

dawehner’s picture

Patch is not ready, there's routes for field and field config, also this affects content translation which routes needs some clean-up... see comment in #2281645-81: Make entity annotations use link templates instead of route names

To be honest I don't get why we want to extract those here as well, given that you have to adapt config translation at the same time as well (yes, believe me).
This issue is for me at least, sorry andypost, a sign of overusing formal issue queue structures/"best" practices.

andypost’s picture

Assigned: andypost » Unassigned

@dawehner I mostly agree, in my vision routes should be renamed in separate issue under the meta
We need to standardize routes without dependency on link templates.

PS: will back to patch next weekend

andypost’s picture

I'd still prefer to separate route rename issues from logic changes

Crell’s picture

So is another patch forthcoming here?

andypost’s picture

dawehner’s picture

@andypost
What is the status of the issue? You assigned it to yourself ... but nothing happened. Do you still work on a new version of the patch?

kgoel’s picture

This doesn't re-roll as patch still applies but testbot will tell. Correct me if I am wrong but config translation can be address in a separate issue so things can move forward.

dawehner’s picture

Status: Needs review » Reviewed & tested by the community

The patch still works, let's get it in.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 3: 2346883-field_ui-routes-3.patch, failed testing.

Status: Needs work » Needs review
andypost’s picture

andypost’s picture

@dawehner is add-form template is needed?

+++ b/core/modules/field_ui/field_ui.module
@@ -299,8 +299,9 @@ function field_ui_entity_type_alter(array &$entity_types) {
+  $form_mode->setLinkTemplate('add-form', 'entity.entity_form_mode.add_form');

@@ -308,6 +309,7 @@ function field_ui_entity_type_alter(array &$entity_types) {
+  $view_mode->setLinkTemplate('add-form', 'entity.entity_view_mode.add_form');

the only missing part in last patch, not sure is needed

dawehner’s picture

We do have at least one call to it:

    $operations['add'] = array(
      'title' => t('Add terms'),
      'weight' => 10,
      'url' => $entity->urlInfo('add-form'),

in VocabularyListBuilder

andypost’s picture

FileSize
1.28 KB
5.48 KB

Once this entities have add forms it makes sense to expose them as templates too

+++ b/core/modules/field_ui/src/EntityDisplayModeListBuilder.php
@@ -119,7 +119,7 @@ public function render() {
       $table['#rows']['_add_new'][] = array(
         'data' => array(
           '#type' => 'link',
-          '#url' => Url::fromRoute($short_type == 'view' ? 'field_ui.entity_view_mode_add_type' : 'field_ui.entity_form_mode_add_type', ['entity_type_id' => $entity_type]),
+          '#url' => Url::fromRoute($short_type == 'view' ? 'entity.entity_view_mode.add_form' : 'entity.entity_form_mode.add_form', ['entity_type_id' => $entity_type]),

This is just a rename

andypost’s picture

Assigned: andypost » dawehner

needs feedback

dawehner’s picture

Status: Needs review » Reviewed & tested by the community

+1

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 20: 2346883-field_ui-routes-20.patch, failed testing.

amateescu’s picture

Status: Needs work » Reviewed & tested by the community
FileSize
5.57 KB

Rerolled.

alexpott’s picture

Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs issue summary update

This issue is a normal task so we need to outline how it fits within the allowable Drupal 8 beta criteria. Can someone add Drupal 8 beta phase evaluation template to the issue summary.

I think this is a followup from the critical work on entity url templates so is allowed.

dawehner’s picture

Issue summary: View changes
Status: Needs work » Reviewed & tested by the community

Added it.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 24: 2346883-24.patch, failed testing.

Status: Needs work » Needs review

andypost queued 24: 2346883-24.patch for re-testing.

andypost’s picture

Status: Needs review » Reviewed & tested by the community
alexpott’s picture

Status: Reviewed & tested by the community » Fixed
Issue tags: -Needs issue summary update

I see this as a followup to #2281645: Make entity annotations use link templates instead of route names and therefore committable under the followup rules of beta evaluation. Committed 275de46 and pushed to 8.0.x. Thanks!

  • alexpott committed 275de46 on 8.0.x
    Issue #2346883 by andypost, amateescu: Standardize field_ui entity route...

Status: Fixed » Closed (fixed)

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