I know this is a bit incomplete. I'm just recording the general issue as a start. See 2717673 for more information.
Problem/Motivation
Prior to 2717673, the calculated dependencies of a comment type did not include the provider module of the target entity. It is likely that some sites have comment types where the provider module has been uninstalled leaving the comment type in an invalid state.
Proposed resolution
Following @catch's suggestion here we need to understand the implications of this and decide on a course of action.
Remaining tasks
Write a test which creates a comment type, uninstalls the provider module and then tests some actions which fail. One action that fails is editing the comment type. That would be a good test to start with.
Create patch.
I don't think we can really "fix" the problem once it has occurred but we should be able to at least avoid fatal errors on the site.
| Comment | File | Size | Author |
|---|---|---|---|
| #14 | missing-entity-provider-2733819-14.patch | 3.73 KB | Munavijayalakshmi |
| #9 | interdiff-2733819-7-9.txt | 2.18 KB | tetranz |
| #9 | missing-entity-provider-2733819-9.patch | 3.61 KB | tetranz |
| #8 | interdiff-2733819-6-7.txt | 2.18 KB | tetranz |
| #8 | missing-entity-provider-2733819-7.patch | 3.61 KB | tetranz |
Comments
Comment #2
tetranz commentedComment #3
tetranz commentedComment #4
catchBumping to major. Could we do a hook_requirements(), find comment types where the target entity type is missing and show a message? Then potentially someone could re-enable the module the uninstalled and recover from there.
Could add a state entry so this only runs occasionally, and not at all once they've all been fixed.
Another option would be deleting those config entities if we can - on the assumption that no live configuration depends on them though, which might be hard to track.
Comment #5
tetranz commentedAdding this test (which will fail) as a start.
I don't quite know where I'm going with this. The suggestion of hook_requirements and a message at some regular interval makes sense so I can look into doing that but ...
This test illustrates what happens if you attempt to edit a comment type after the provider has been uninstalled. The user gets the white screen with "The website encountered an unexpected error". Do we want to do something about that? If so what? It seems like it should not crash but give the same message as triggered by hook_requirements. I'm not sure how to approach that. The comment type edit page is the default entity edit form.
Comment #6
tetranz commentedHere's a patch and test for hook_requirements. It puts a warning message on the status report and the update page.
@catch, I'm now a bit confused by your suggestion of using state to show the message occasionally. hook_requirements only runs at install, update and the status report. Do you mean a drupal_set_message() type message that appears occasionally for the admin?
I'm unsure if I've chosen a correct or appropriate name for the test. It's a bit long and awkward. Likewise for the wording of the warning.
Comment #7
tetranz commentedComment #8
tetranz commentedImproved the test.
Comment #9
tetranz commentedFixed some typos in the comments.
Comment #11
tetranz commentedThat MigrateUpgrade7Test fail really does seem random.
I see one more silly typo in my comments (article comment type should say content) but I'll wait for a review before bothering the test bot again :)
Comment #14
Munavijayalakshmi commentedRerolled the patch.
Comment #24
smustgrave commentedThis issue is being reviewed by the kind folks in Slack, #needs-review-queue-initiative. We are working to keep the size of Needs Review queue [2700+ issues] to around 400 (1 month or less), following Review a patch or merge request as a guide.
At this time we will need a D10.1 of this patch.
Also tagging for subsystem review on their thoughts.