Started a MR with an initial proposal. I don't love it but it's the best I could come up with :)
I think RTBC is the correct status when we have the backport MR ready.
Here it is: 📌 Optimize database writes when re-saving a pending revision as the default one Active
@smustgrave, yup, and the test for it is in ::testMoveAllTrackedEntities() :)
I'm seeing the deprecations are changing in WorkspaceManager but believe these changes have already been in a release so is that okay?
I think it is. BC for constructors is done on a best-effort basis, and I was actually surprised I was able to do this change while preserving full BC :) The change here just means that classes extending WorkspaceManager that were updated for 11.2 will need to be updated again for 11.3 before 12.0.0 lands.
I see we are switching to cron and limiting to 50, which makes sense. My question is if those workspaces are large with lots of content each any concern using cron then?
The method we're deprecating was already ran by cron, we just moved the code around so there's no actual behavior change.
Do we need a CR for the new Event and Subscriber?
Right, I was waiting for the review thread for it to reach a conclusion. Added an additional CR: https://www.drupal.org/node/3553871 →
Here's the initial implementation issue: #3553775: Add a conflict management API →
Wondering, what is the planned upgrade path to populate the "provider" field?
It's already populated by workspaces_update_11303() with ->setInitialValue(DefaultWorkspaceProvider::getId()) :)
This is a very interesting proposal. We'll need to carefully consider the implications for it though..
@smustgrave, the issue summary says that we're not yet introducing the UI for this, that's why you couldn't find it :)
Anyway, this is blocked by 📌 Slim down WorkspaceManager Needs review now, so postponing.. hopefully for a short while.
Replied to the latest comments, and left the thread about checkAccess() open for core committers to give an opinion :)
Answered the new comments, thanks for being so thorough! :)
@geek-merlin, thanks a lot for the in-depth review! Addressed all points and wrote the change records.
Live-debugging this helped a lot :) Merged into 3.x, thanks!
After trying out the initial proposal here with a temporary field, I found that it's too limiting (as @geek-merlin also mentioned above), and that we need to go a few steps further and allow providers to easily control the behavior of entity operations in a workspace.
📌 [PP-1] NodeRouteProvider should extend DefaultHtmlRouteProvider Needs work got in, so this is no longer blocked.
We already have a view for displaying all entities in the trash.
We don't have a view, the listing pages at /admin/content/trash are generated "manually" in \Drupal\trash\Controller\TrashController::listing :)
@twod, can you please check if the change from 🐛 Update from 3.0.18 to 3.0.20 breaks D10 site with Draggable views Active fixed the problem for you as well?
QueryPluginBase::getEntityTableInfo() is a very nice find, exactly what we were missing :) Merged into 3.x, thanks!
I'll wait a bit before cutting a new release, maybe someone else can provide at least one more confirmation.
The MR is written on top of 📌 Add the ability to move content between workspaces Needs review , so postponing on that.
Decided to add the new method on the interface under the 1-1 rule, so we don't have to block other work on the followup.
Paragraphs shouldn't be tracked by workspaces. You should run drush php:eval "\Drupal::moduleHandler()->loadInclude('wse', 'install'); _wse_update_clean_entity_types_workspace_metadata(['paragraph']);" and then manually clean up any records in the workspace_association table that references paragraph entities.
The current MR works well for its purpose, but we're planning on adding a full admin UI for preview links. Sebastian is working on that so assigning it to him.
Note that this issue is closed, because it's not a problem in the WSE module. See #5.
Added a comment to explain why we also need to it here. This new ::setNewRevision() call is needed for the specific case of adding a new published entity in a workspace.
What else am I supposed to do other than run entity updates?
Investigate why there are no database updates to run. In most cases this means you had a patch that provided an update function with the same number.
but every redirect on the site is disabled
Do you have a backup of the dev database _before_ running drush updb? If so, can you check the redirect table and see if there's an enabled field already?
@ronraney,
drush updbdidn't do anything when I tried that
This probably means you previously had a patch applied which had an update function named redirect_update_8110(). You can follow the steps I detailed here to fix it:
#3543872-2: Latest version of Redirect module failed to run updates. →
Also, drush entup is never the solution to use for a production site, that's supposed to be used only in development when there's no entity data.
Closing this issue for now, but please reopen if you manage to reproduce it and provide more info.
I've been using Trash on a large site that uses Groups a lot and didn't bump into any problems, so unfortunately this one will need some more debugging on your end.
I think there was a problem with post_execute but I can't really remember :/ Maybe I just wanted to be sure that we restore the trash context as late as possible to account for any code that might deal with deleted entities..
I'll need to check if there's another way to do this at some point.
This is no longer needed after ✨ Convert Redirect entity to revisionable Needs work .
Yup, that's the fallback for any db that doesn't support it.
@smustgrave, I don't see how this can impact contrib, it's only adding functionality :)
While this patch can be used for sites that need it, I'd prefer waiting for ✨ Translation support Needs review to be finished instead of introducing more workarounds..
Also, uninstalling the module is not required, you can just disable trash for the specific entity bundle that you're dealing with. That way, the deleted field is not gone, and you can re-enable it afterwards.
That's exactly the purpose of \Drupal\wse\Plugin\Block\SwitchToLiveBlock :)
I couldn't figure out why some of the original code was written as it was.
Looking at the issue where I wrote that code ( 🐛 Trash breaks revision-based views and relation queries Fixed ).. I can't figure out either :) But the change in this MR makes a lot of sense, we surely don't need to loop over all possible field table names of the deleted field if we're already looping through all the tables involved in the views query.
I also tested a revision view with the same setup described in the issue summary and it works fine.
Merged into 3.x, thanks!
Opened 🐛 Language-neutral path aliases are not deleted with the parent entity Active to fix a regression introduced by this issue.
This was fixed in
#2233595: Deprecate the custom path alias storage →
where PathFieldItemList uses a single alias repository query in ::computeValue().
Decided to add full support path aliases instead: 📌 Add support for path aliases Needs review
The reason why WSE removes those operations is that it needs to enforce core's (current) workspace restriction policy: once you start editing an entity in a workspace, you can only interact with it in that workspace until it gets published to Live.
Since Parallel Workspaces removes core's restriction, I think it should also allow those two revision operations :)
amateescu → changed the visibility of the branch 3548279-add-workspae-task-monitoring to hidden.
Do file and media entities still have to be excluded after the Twig fix?
And should we attempt to remove a similar optimization from \Drupal\menu_link_content\Plugin\Menu\MenuLinkContent::$entityIdsToLoad here or in another issue?
Looking at function redirect_update_8110(), it does add the field and set the default value for the field -- but THIS ONLY AFFECTS NEW VALUES GOING FORWARD -- it does not set the value on existing redirects.
That's incorrect. redirect_update_8110() sets the initial value for the field, see line 258: https://git.drupalcode.org/project/redirect/-/blob/8.x-1.x/redirect.inst...
Are you sure the update function ran correctly? Or was the devel_entity_updates module used to "fix" the missing new field?
The core implementation of these methods will be vastly improved in Drupal 11.3 ( 📌 Refactor workspace association tracking to use a dedicated revision table Active ), so I think it's fine to revert them from WSE.
Merged into 2.0.x, thanks!
Opened a sibling issue in the Trash queue: 📌 Integration with the Redirect module Fixed