Closed (fixed)
Project:
Configurable Help
Component:
Code
Priority:
Normal
Category:
Task
Assigned:
Reporter:
Created:
11 Nov 2017 at 20:21 UTC
Updated:
25 Nov 2017 at 22:44 UTC
Jump to comment: Most recent, Most recent file
Problem/Motivation
Core using following patters to manage config entities
- admin_path/add - create
- admin_path/manage/{id} - edit
- admin_path/manage/{id}/delete - delete and so on..
See vocabularies/views...
For content entities this actions are
- entity_type/{id} - view
- entity_type/{id}/edit - edit and so on
Proposed resolution
As #2372585: Change order of tabs to View / Edit / Translate fixed better to adopt common pattern for config entities
| Comment | File | Size | Author |
|---|---|---|---|
| #5 | 2922774-5.patch | 7.65 KB | jhodgdon |
Comments
Comment #2
andypostThis will allow to get rid of hack with "add" in
\Drupal\config_help\Form\HelpTopicForm::exists()(which I guess was picked from old taxonomy implementation and where the path patters were introducedComment #3
jhodgdonGood idea, let's adopt the standard config entity pattern.
Comment #4
jhodgdonActually, I can do this one today. I don't think it will interfere with any of the other issues we're working on (and can save you a few lines in the autocomplete issue).
Comment #5
jhodgdonHere's a patch. I'm running the tests locally... meanwhile, I was unsure.. should we change the view path also? Most config entities don't have a view path, so it's not clear to me whether we should change it. Changing it to manage/{id} seems weird, because viewing isn't managing.
It seems like most content IDs are auto-assigned numbers, so they have no conflict with the "add" page...
Thoughts?
Comment #6
andypostThat looks great, now next 4 lines could be removed http://cgit.drupalcode.org/sandbox-jhodgdon-2369943/tree/src/Form/HelpTo...
Comment #7
jhodgdonTests passed locally...
Comment #8
andypostmaybe add a test for "add" machine name?
Comment #10
jhodgdonOK, I will go ahead and commit this to the sandbox, update the Core patch on parent issue, and let you deal with the 4 lines removal, since you are working on autocomplete, OK?
Thanks for the quick review!
Comment #11
jhodgdonWe can't add a test for the "add" machine name until those 4 lines go away in the Form class... I'm not too worried about it though, it's kind of an edge case.