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.
Revealed via #1373142: Use the Testing profile. Speed up testbot by 50%
Problem
- Renaming a node type bundle causes notices on manage fields page (and fields on the bundle are likely not moved).
Reproduce
- drush -y si minimal; drush -y en field_ui
- visit 'admin/structure/types/add', create content type
- edit content type from 2., just change the machine name and save
- visit the manage fields page for the content type from 2.
- FAIL, notice as in the test.
Attached patch has a test that does that, and throws the test exception.
Looks like some random dependency on something that amounts to a missing field_cache_clear() call.
Related issues
Comment | File | Size | Author |
---|---|---|---|
#37 | 1375452-37-tests-only.patch | 2.77 KB | dcam |
#37 | 1375452-37.patch | 4.37 KB | dcam |
#32 | 1375452-32.patch | 1.17 KB | swentel |
#30 | 1375452-30.patch | 1.09 KB | swentel |
#29 | 1375452-29.patch | 1.05 KB | swentel |
Comments
Comment #1
Anonymous (not verified) CreditAttribution: Anonymous commentedComment #2
catchThis sounds similar to #1079966: Notice: Undefined index: [custom_content_type_machine_name] in _field_ui_bundle_admin_path(), line 309 in field_ui.module .
Comment #4
sunThanks @catch for digging out that issue -- it's pretty obvious now.
Comment #5
sunApparently, #4 does not fix the bug, so we're back to square one.
However, clearing the entity info cache seems like a good idea either way, since bundles are also cached there, no?
Comment #6
Anonymous (not verified) CreditAttribution: Anonymous commentedthis bug is caused by stale info from _node_types_build().
when we clear the entity info cache, the next call to entity_get_info calls into module's hook_entity_info implementations.
node_entity_info calls in to node_type_get_name, which returns stale info right back into our freshly rebuilt entity info. awesomeness.
Comment #7
sunFirst of all, attached patch exposes the bug.
Comment #8
sunI guess this would be the most simple solution, err, stop-gap fix.
Comment #9
Anonymous (not verified) CreditAttribution: Anonymous commentedyes, i think we need to go with the simple fix for now, then open follow up patches or link to existing cleanups in the entity system.
this is a simple patch, with a test, i think its RTBC.
Comment #10
catchI've opened #1376884: Use configuration for entity types ;)
This looks good to me, comes with a nice test, and fixes a bug exposed by testing cleanup. Committed/pushed to 8.x.
Comment #11
Anonymous (not verified) CreditAttribution: Anonymous commentedand here's the D7 version.
Comment #12
Anonymous (not verified) CreditAttribution: Anonymous commentedalso, look at the lists of likely related issues in #686938-61: Undefined index: admin in _field_ui_bundle_admin_path() ... ewwww.
Comment #13
pjcdawkins CreditAttribution: pjcdawkins commentedThe patch in #11 works for me, but I haven't tested the test.
Comment #14
oriol_e9gGo!
Comment #15
droplet CreditAttribution: droplet commented#11: rename-bundle-1375452-11.patch queued for re-testing.
Comment #17
oriol_e9gRerolled. Two patches, one with only the tests and other with all.
Comment #18
oriol_e9gComment #19
oriol_e9gSame with small whitespaces fix.
Comment #20
Anonymous (not verified) CreditAttribution: Anonymous commentedthanks oriol_e9g, lets see what webchick thinks.
Comment #21
webchickConfirmed that the patch clears up the bug.
The one thing I'm not as crazy about though is the test. While I can confirm that without the rest of the patch, the test fails on its own, I'm not able to reproduce it in the minimal profile by following the steps (Create a content type, a vocabulary, and a term reference field). Versus I can clearly trigger the bug with beejeebus's steps in the OP, and "rename bundle" seems like a worthy thing to have a test for.
The comments in the test also don't clear things up for me as to why this actually triggers anything because they say:
...and? :) What's the punchline? What's the condition are we checking for?
So if we could either re-work the tests to be more along the lines of beejeebus's steps in #1, *or* more clearly articulate in these tests what we're testing here (with an assertion or similar) that would really help.
Comment #22
sunThere is a FieldUIManageFieldsTestCase::testRenameBundle() test already.
However, the test only reveals that the code throws an exception with the entity_info_cache_clear() from #7.
The lines outlined in #21 are technically irrelevant for this issue - they merely make FieldUIManageFieldsTestCase work with the Testing profile (the master patch revealed this bug).
That said, we could improve testRenameBundle() to test the expected results a bit more explicitly. All it currently does is:
...without any assertions after each of those requests.
At the very least, we could assert confirmation messages, and response status codes. That said, the drupalGet() always yields a positive 200 response, because the menu router automatically falls back to
admin/structure/types
, in case the %node_type cannot be found - so the test should verify that the GET actually shows the Manage fields page of the renamed node type.Comment #23
catchThis is just test fixes now, downgrading status.
Comment #24
Tanrry868 CreditAttribution: Tanrry868 commented#19: rename-bundle-137452-19.patch queued for re-testing.
Comment #26
drupalycious CreditAttribution: drupalycious commented#19: rename-bundle-137452-19.patch queued for re-testing.
Comment #28
drupalycious CreditAttribution: drupalycious commentedHello,
as the patch failed testing, is someone interested to modify it?
Thanks
Comment #29
swentel CreditAttribution: swentel commentedThis should do it.
Comment #30
swentel CreditAttribution: swentel commentedActually, this is even better.
Comment #31
yched CreditAttribution: yched commentedWorks for me, but would need a phpdoc block on ManageFieldsTest::manageFieldsPage() then.
Comment #32
swentel CreditAttribution: swentel commentedComment #33
yched CreditAttribution: yched commentedThanks, works for me.
Comment #34
webchick#32: 1375452-32.patch queued for re-testing.
Comment #35
webchickOops, sorry. That was me sucking at Git. :P
Committed and pushed to 8.x. Thanks!
Moving to 7.x for backport.
Comment #36
lias CreditAttribution: lias commentedHas this been backported? I've got the latest drupal version, 7.21, installed and I'm still getting these errors.
Comment #37
dcam CreditAttribution: dcam commentedBackported #8 and #32 to D7.
Comment #38
corbin CreditAttribution: corbin commented#37: 1375452-37.patch queued for re-testing.
oops, sorry.
The patch doesn't seem to work for kickstart;
"doesn't seem" because maybe I made another error :-(
Comment #39
saurabh tiwari CreditAttribution: saurabh tiwari commented#37: 1375452-37.patch queued for re-testing.
Comment #40
dags CreditAttribution: dags commented#37 did not fix the issue for me using Drupal 7.23. I encountered the error after enabling the core Blog module and then disabling it. Patch #7 in #1898110: Notice: Undefined index: blog provides a work around for the issue.
Comment #40.0
dags CreditAttribution: dags commentedUpdated issue summary.
Comment #41
kopeboy CreditAttribution: kopeboy commentedStill getting a lot of these errors at Reports > Field list, on Drupal 7.31, related to:
Blog
Forum
Comment
Commerce Node Checkout
(all uninstalled apart from Comment)
and a content type I renamed.
Any update?
Still needs code fixes or should I just re-enable and re-uninstall the modules?
What about the content type (which now I don't remember which one was?)
Thanks
Comment #42
jessZ CreditAttribution: jessZ commentedRunning 7.31 with get this error. If enable forum and comments, errors refer to comments instead.
Notice: Undefined index: forum in _field_ui_bundle_admin_path() (line 325 of /home/reimagi8/public_html/new/modules/field_ui/field_ui.module).
I also get this error with a number pf content types that were deleted in drupal 5 and 6. (this site has survived upgrade from 4.7 > 7.3
After i deleted alot of unneeded field types that showed up correctly i was able to run database update with nor errors but up until then i was getting a dependency warning when trying to update the database.