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.
Problem/Motivation
Running database updates via the UI while having an active workspace in the current user's session leads to all kinds of interesting failures. One example is that post update functions can not save config entities because that's an unsupported operation in the context of a workspace.
Proposed resolution
Ensure that \Drupal\workspaces\WorkspaceManager::getActiveWorkspace()
doesn't return a workspace object while running database updates.
Remaining tasks
Review.
User interface changes
Nope.
API changes
Nope.
Data model changes
Nope.
Release notes snippet
Nope.
Comment | File | Size | Author |
---|---|---|---|
#6 | 3093879-6.patch | 6.7 KB | amateescu |
#6 | 3093879-6-test-only.patch | 5.49 KB | amateescu |
#2 | 3093879.patch | 8.09 KB | amateescu |
#2 | 3093879-test-only.patch | 3.48 KB | amateescu |
Comments
Comment #2
amateescu CreditAttribution: amateescu for Pfizer, Inc. commentedThis should do it.
Comment #3
catchIs this the current way to figure out if we're in an update or not? I can't see similar anywhere else yet.
Don't we also need to do this in the migrate UI too for similar reasons?
Comment #6
amateescu CreditAttribution: amateescu for Pfizer, Inc. commentedThanks for the review!
That was my initial thought about handling this, but, after sleeping on it for a bit, I think a better way to do this is to ensure that no workspace negotiators run during database updates, similar to how we handle path aliases in
\Drupal\Core\Update\UpdateServiceProvider::alter()
.The migration part of this problem is being handled differently in #3052115: Mark an entity as 'syncing' during a migration update.
Also figured out a better way to test this, so here's a fresh set of patches.
Comment #7
catch#6 looks a lot more like I was expecting.
Comment #9
BerdirPreviously, it would have made sense to make sure that we do have a value, but assertTrue/False are now type sensitive, so assertFalse(NULL) will fail, so that's good.
This looks good I think.
Comment #15
catchCommitted/pushed to 9.0.x, 8.9.x, 8.8.x, thanks!
Comment #20
jibranI'm making the following change over in #3087644: Remove Drupal 8 updates up to and including 88** and it is complaing about no update hooks whereas test module with post-update hook is present and installed. Any quick pointer here or I'd debug in more detail.