Problem/Motivation

Currently taxonomy use custom admin/structure/taxonomy/{vocabulary}/edit path that does not allow use of add/edit names and have a test for that.
Other core parts uses admin/.../{entity_type}/manage/{id} path

Proposed resolution

Change path for all vocabularies to admin/structure/taxonomy/manage/%taxonomy_vocabulary/edit

#843162: Creating vocabularies with machine-names "List" or "Add" breaks links in taxonomy overview admin area.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

andypost’s picture

Status: Active » Needs review
FileSize
25.08 KB

Trying to keep it simple

andypost’s picture

FileSize
25.14 KB
635 bytes

missed hunk

Status: Needs review » Needs work

The last submitted patch, 1978112-taxonomy-path-2.patch, failed testing.

andypost’s picture

Status: Needs work » Needs review
FileSize
32.53 KB
7.09 KB

Fix the rest places

larowlan’s picture

Status: Needs review » Needs work
+++ b/core/modules/shortcut/shortcut.api.phpundefined
@@ -33,7 +33,7 @@
-    return 'admin-shortcuts';

unrelated?

Apart from that looks good.

andypost’s picture

Status: Needs work » Needs review
FileSize
32.02 KB
andypost’s picture

#6: 1978112-taxonomy-path-6.patch queued for re-testing.

ParisLiakos’s picture

Status: Needs review » Needs work

consistency++
but needs a reroll..otherwise looks good

andypost’s picture

Status: Needs review » Needs work

The last submitted patch, 1978112-taxonomy-path-6.patch, failed testing.

andypost’s picture

Status: Needs work » Needs review
FileSize
32.48 KB

Once other one needs work let's normalize path

dawehner’s picture

Status: Needs review » Reviewed & tested by the community
+++ b/core/modules/taxonomy/lib/Drupal/taxonomy/VocabularyFormController.phpundefined
@@ -108,26 +108,6 @@ protected function actions(array $form, array &$form_state) {
   /**
-   * Overrides Drupal\Core\Entity\EntityFormController::validate().
-   */
-  public function validate(array $form, array &$form_state) {
-    parent::validate($form, $form_state);
-
-    // Make sure that the machine name of the vocabulary is not in the
-    // disallowed list (names that conflict with menu items, such as 'list'
-    // and 'add').
-    // During the deletion there is no 'vid' key.
-    if (isset($form_state['values']['vid'])) {
-      // Do not allow machine names to conflict with taxonomy path arguments.
-      $vid = $form_state['values']['vid'];
-      $disallowed = array('add', 'list');
-      if (in_array($vid, $disallowed)) {
-        form_set_error('vid', t('The machine-readable name cannot be "add" or "list".'));
-      }
-    }
-  }

Awesome! I went through the changes and every single line looked perfect!

andypost’s picture

webchick’s picture

Title: Convert taxonomy admin path to follow other core » Change notice: Convert taxonomy admin path to follow other core entity patterns
Priority: Normal » Critical
Status: Reviewed & tested by the community » Active
Issue tags: +Needs change record

I was a bit alarmed to see a big hunk of validation logic removed with no replacement until I realized how totally effing janky said validation is and I'm really glad we don't need it anymore. ;)

Committed and pushed to 8.x. Thanks!

Wouldn't hurt to have a change notice for this, for modules whose documentation or whatnot might be pointing to the old URLs.

LittleCoding’s picture

Assigned: Unassigned » LittleCoding

Write a API change notification

LittleCoding’s picture

Status: Active » Needs review

Summary

The Drupal taxonomy admin path was converted to follow other core entity admin patterns with the addition of "manage" to the admin path.

Drupal 7

admin/structure/taxonomy/%taxonomy_vocabulary/edit

Drupal 8

admin/structure/taxonomy/manage/%taxonomy_vocabulary/edit
LittleCoding’s picture

A change record node has been created

http://drupal.org/node/1986784

larowlan’s picture

Assigned: LittleCoding » Unassigned
Priority: Critical » Normal

Changes record looks good

star-szr’s picture

Title: Change notice: Convert taxonomy admin path to follow other core entity patterns » Convert taxonomy admin path to follow other core entity patterns
Status: Needs review » Fixed
Issue tags: -Needs change record

This is fixed now, reverting tags and title. Thanks @LittleCoding!

Status: Fixed » Closed (fixed)

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

Anonymous’s picture