Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Hi,
I've just installed the d8 branch using simplytest.me with the required patch to have a look at the most recent changes. When I clicked through the menu points (without changing anything in the standard setup) I noticed that on admin/structure/contact you cannot edit a contact form category. Neither can you use the links provided for 'managing fields' or 'managing display'.
Deleting on the other hand works as expected.
I then created a new category without problems. The new category showed the same behavior.
Can somebody confirm this issue?
Comment | File | Size | Author |
---|---|---|---|
#23 | interdiff.txt | 1022 bytes | ParisLiakos |
#23 | config-list-2010290-23.patch | 16.41 KB | ParisLiakos |
#19 | config-list-2010290-19.patch | 16.4 KB | tim.plunkett |
#19 | interdiff.txt | 953 bytes | tim.plunkett |
#17 | config-list-2010290-16.patch | 15.84 KB | tim.plunkett |
Comments
Comment #1
larowlanTagging
Comment #2
sergeypavlenko CreditAttribution: sergeypavlenko commentedI confirm, today installed drupal 8, and has the same problem.
Comment #3
larowlanTackling
Comment #4
larowlanFail then pass
Comment #5
ParisLiakos CreditAttribution: ParisLiakos commentedthis is definitely critical
Comment #6
BerdirVarious things are also getting fixed in #1983548: Convert contact message entities to the new Entity Field API, including at least the manage fields/display, without test coverage for that though. contact.module is hopelessly broken right now ;)
Comment #7
ParisLiakos CreditAttribution: ParisLiakos commentedrelevant #1938386-17: Convert contact_category_* pages to a new-style Controller
Comment #8
alexpottOkay this issue does not affect just contacts it also affects roles. The issue is a direct result of #1995620: [policy, no patch] Document how to handle routes for MENU_DEFAULT_LOCAL_TASK.
Totally different patch to #4
And we obviously have absolutely no tests for this in atm...
Comment #10
larowlanThanks, will combine your fix with my tests asap
Comment #11
tim.plunkett#8 was 100% the right fix.
I was overriding this as I went in my conversions, not sure what happened with contact.
Menu was #2019735: operatuins typo in MenuListController.php, closing that.
Here's the fix from #8, the test from #4, and my fixes and clean-ups.
We should really go through and change more of these web tests to use clickLink().
Comment #13
tim.plunkettContact categories are not fieldable, so they shouldn't have provided links to Field UI code.
#1983548: Convert contact message entities to the new Entity Field API seems like it might change that, but that can be done there.
Unlike all other config entities, the default task for Vocabulary is listing, and /edit is a local task.
Comment #15
tim.plunkettI just broke shortcuts, so maybe we need tests after all.
Comment #16
larowlanContact messages are fieldable, that's what the links were for.
Comment #17
tim.plunkettFixed shortcut, added a test there too.
Comment #19
tim.plunkettCould have sworn I fixed that one.
Comment #20
tim.plunkettJust realized I was fixing many of the same bugs in #2006348: Remove default/fallback entity form operation, so postponing that.
Comment #21
tim.plunkettComment #22
ParisLiakos CreditAttribution: ParisLiakos commentedthat was fun:) config entities dont have fields, i always wondered why contact categories had the manage fields link in the dropbutton:P
thanks for restoring some sanity there:)
this could be cleaner, also i cant see where you use the $name after that
Comment #23
ParisLiakos CreditAttribution: ParisLiakos commentedwell, there, lets make it beautiful:P
Comment #25
ParisLiakos CreditAttribution: ParisLiakos commentedrandom failure #2017217: Random failure on UserPasswordResetTest
#23: config-list-2010290-23.patch queued for re-testing.
Comment #26
larowlanContact Message entities are fieldable, that hunk needs to go back in
Comment #27
tim.plunkettThat hunk was in CategoryListController. Which is used by \Drupal\contact\Plugin\Core\Entity\Category. Also, it doesn't work.
\Drupal\contact\Plugin\Core\Entity\Message doesn't specify a list controller.
Neither of them specify a route_base_path.
Back to RTBC before the random fail.
Comment #28
larowlanAren't Contact messages content entities?
The route base path missing was part of the earlier fix and test.
Happy to be wrong here
Comment #29
larowlan#1825044: Turn contact form submissions into full-blown Contact Message entities, without storage
Comment #30
ParisLiakos CreditAttribution: ParisLiakos commentedseems we are confused with categories and messages:)
Contact messages are fieldable, Contact categories are not
Comment #31
tim.plunkettCategoryListController::getOperations()
It gets passed an $entity which is a CategoryInterface.
That is a 404, since categories are not fieldable, \Drupal\field_ui\Routing\RouteSubscriber doesn't provide a route for them.
Message is a fieldable entity, but it does not provide a route_base_path, so it also is ignored by Field UI.
Comment #32
tim.plunkettOkay, so what happened here was @larowlan was trying to fix contact.module.
His initial patch had a bunch of fixes, like adding route_base_path.
I missed that completely when I was pinged by @alexpott to look at this, and went off of #8, which was a more generic bug.
So yes, Contact Messages are supposed to be fieldable, and this whole confusion now make sense.
But we've polluted the issue with a generic fix for a separate bug.
We can split one off, not sure if anyone cares which one.
Comment #33
alexpottCommitted 096b8c5 and pushed to 8.x. Thanks!
Comment #34
BerdirA bit confused that the patch was now committed :)
As @larowlan said above, contact messages *are* fieldable, it's just the field UI integration that's broken (together with various other things), so there's no reason to remove those links, they should simply be fixed.
I already partially fixed it in #1983548: Convert contact message entities to the new Entity Field API, should I merge the non-committed things from here into my patch there, including test coverage?
Comment #35
tim.plunkett@Berdir, larowlan said he'd file a new issue for the previous fix/tests, but rolling that into the above issue could be okay too.