🇷🇴Romania @amateescu

Glad to hear that, we can close this now.

I've committed the patch in #5 to 3.x, but I'm keeping the issue open in case there are any more details that can be shared.

Here's a patch that sets a default value for the entity_update_batch_size setting, which might not exist in a site. Also added a safety check if there are no items to purge.

However, the auto_purge.enabled trash setting is false initially, and the only entity type that's enabled by default is node.

@CNDexter, are you sure that the error happened when installing the module? And if so, can you check if the file entity type already has a deleted field?

Merged, thanks!

I've committed https://git.drupalcode.org/project/wse/-/commit/5262ffb76c4b975a2c2d862c... and opened the 2.0.x branch for D10.3+ compatibility. Also posted a helpful table on the project page.

git diff 1.0.x --stat shows how much Workspaces evolved in Drupal 10.3 :)

75 files changed, 242 insertions(+), 3056 deletions(-)

As you learned already, the current codebase is very much incompatible with Drupal 10.3. I just marked it as such with this commit: https://git.drupalcode.org/project/wse/-/commit/473d11be143177c230ae164f...

I'll open a 2.0.x branch for 10.3+ compatibility because it's too hard to keep it working with lower versions.

Quoting @catch in #71:

My only (or at least main) objection is to storing things that are entity content as static props instead of field-backed, something which has already been implemented and is also being explicitly argued for in this issue and elsewhere.

This is also my main concern with the JSON-based storage proposed in this issue, and I'd really like us to arrive to a solution that stores most* data in individual entity fields. The goal of removing the indirection layer that Layout Builder and Paragraphs have with referenced entities is very much worthwhile, but going into the direction of storing everything in a blob is not much of an improvement IMO.. quite the contrary.

* By "most" I mean that the majority usage pattern for building 'layout pages' is to fill data for a predefined list of components (at least in my experience), while one-off components like the "Christmas hero" example mentioned above are the exception, and I hope we can find a way to store that data without having it "dictate" the entire storage model.

Thanks :)

Did you run the database updates after upgrading to 3.0.4? trash_post_update_set_enabled_entity_types_bundles() updates the trash.settings configuration to the (new) structure required by getEnabledEntityTypes().

@Wim Leers, re #52:

Renumbering is the high-risk piece (i.e. the weak link) in this approach.

Note that \Drupal\Core\Field\FieldItemList::setValue() is not set in stone, field types can specify a custom ItemList class where that method can be overridden to support the use-case of assigning various field deltas to different components. Also, the other place that does renumbering is \Drupal\Core\TypedData\Plugin\DataType\ItemList::rekey(), and that can be overridden too :)

FWIW, I added some `sleep()` calls in that code, executed drush cr and drush updb -y from the CLI, then browsed the site while the commands were still running, and I didn't bump into that error.

However, if you find that the MR makes a difference in your case, I've no problem with merging it :)

Maybe this additional check would help...

@mparker17, did you somehow run trash_post_update_set_enabled_entity_types_bundles() twice by any chance? That would be a situation where the enabled_entity_types property of trash.settings would become an empty array, and \Drupal\trash\EventSubscriber\TrashConfigSubscriber::onSave() would then remove the deleted field from all previously enabled entity types.

I've tried writing a custom filter similar to the one posted in #9 using "standard" entity reference fields, but I wasn't able to reproduce the situation where a field table was considered an entity base table.

This is the filter I tried:

/**
 * @ingroup views_filter_handlers
 *
 * @ViewsFilter("trash_test")
 */
class TrashTest extends FilterPluginBase {

  public function query() {
    // Join from daily availability entity to bookable entity (the actual room).
    $configuration = [
      'table' => 'node__field_tags',
      'field' => 'entity_id',
      'left_table' => 'node__field_image',
      'left_field' => 'entity_id',
    ];

    $join = Views::pluginManager('join')->createInstance('standard', $configuration);
    $rel = $this->query->addRelationship('node__field_tags', $join, 'node');

    // Join from bookable entity to loge entity.
    $configuration = [
      'table' => 'media',
      'field' => 'mid',
      'left_table' => 'node__field_media',
      'left_field' => 'field_media_target_id',
    ];

    $join = Views::pluginManager('join')->createInstance('standard', $configuration);
    $rel = $this->query->addRelationship('media', $join, 'media');

    $this->ensureMyTable();
  }

}

and defined like this in \Drupal\node\NodeViewsData:

    $data['node_field_data']['trash_test'] = [
      'title' => $this->t('Trash test'),
      'help' => $this->t('ABCD.'),
      'filter' => [
        'id' => 'trash_test',
        'label' => $this->t('XYZ'),
      ],
    ];

FWIW, this MR missed adding an upgrade path for the new auto_purge configuration, I've just added it in https://git.drupalcode.org/project/trash/-/commit/42664da3d2d8e64702cb16...

This was fixed in 🐛 Autocomplete dragtable allows empty values and inserts empty entry Fixed .

Rebased and merged into 8.x-1.x, thanks!

amateescu → made their first commit to this issue’s fork.

Needs a rebase.

This was eventually done in ✨ Implement core action plugins to support ECA, others Fixed , so I credited you over there. Sorry for taking so long to look at this issue..

Crediting @minorOffense for his work in #3215196: Add action plugins for subqueue operations (D9) → .

Merged into 8.x-1.x, thanks!

I've moved @akalam's commit into a separate issue: 🐛 Display only the operations the user has access to in subqueueListForEntity Fixed

@artemboiko, can you please write some steps to reproduce for this part?

we have a bug with configure user don't have access to this page but still can see link on entityqueue list page

Also, what's the use case for a separate enable/disable permission? Is the general "update" permission not sufficient?

Committed to 8.x-1.x, thanks!

Bringing over the commit from https://git.drupalcode.org/project/entityqueue/-/merge_requests/20/diffs... into this issue.

amateescu → created an issue.

Committed to 8.x-1.x.

Good catch, here's a patch for this problem.

Fixed in 📌 Automated Drupal 11 compatibility fixes for Entityqueue Fixed .

Merged into 8.x-1.x, thanks!

The simplified workspace switcher has been rewritten and it seems to work well enough now.

Cleaned up the code a bit and merged into 1.0.x, thanks!

amateescu → made their first commit to this issue’s fork.

Oops sorry about that, I didn't look at the credit field when I merged it. Done :)

@jwwj, is it possible to post the code of that custom filter in a way that doesn't expose any site-specific information? I think I understand the problem now, but I need to be able to replicate it locally in order to work on a fix :)

This looks great! Fixed the tests and did a few small tweaks, and merged into 3.x, thanks!

amateescu → made their first commit to this issue’s fork.

All done!

amateescu → created an issue.

No new feedback from the bot and D11 beta is out, so I think this issue can be closed.

I've removed the custom implementation of SetInlineBlockDependency from wse_lb because it was assuming a specific workflow, where layouts are only edited in a workspace, and it wasn't really needed as well, setting the inline block dependency by revision ID works just fine in a workspace.

This patch is no longer needed, so I'm closing this issue.

Linking the issue mentioned by @catch above.

Looks good to me :)

Merged into 1.0.x, thanks!

It seems this small change won't make it into this module, the core issue in Workspaces is getting fixed though, so closing this one.

This is actually a bug because in HEAD, if you have Layout Builder enabled and edit an entity in a workspace, when you visit `/node/X/layout` in Live, the concurrent editing message is not displayed. Added a test for this problem and keeping the issue at RTBC because it's a simple test addition with no other functional change to the MR.

Added a basic token validation for the query parameter workspace negotiator to comply with the interface description of the new method.

Ok, let's do this, the change is harmless and allows the class to be instantiated :)

@stefan.korn, I replied in #3411308-10: WorkspaceSubscriber service parameter $workspaceAssociation must be optional → .

In the service definition, the workspaces.association service must be marked as optional because it might not exist (for example when the Workspaces module is not enabled).

Note that this is not a bug because \Drupal\content_moderation\EventSubscriber\WorkspaceSubscriber is an event subscriber class, and it shouldn't be instantiated unless it needs to react to the WorkspacePrePublishEvent event, which is only invoked when the Workspaces module is enabled. If some other module instantiates it.. well, it shouldn't :)

If we want to make a change in core though, we need to mark the argument as optional in \Drupal\content_moderation\EventSubscriber\WorkspaceSubscriber::__construct(). Updated the issue summary to reflect that.

Discussed a bit with @catch and I'll do a few changes to the MR.

@longwave, good idea, done!

Well.. the MR doesn't change any functionality in Views, so I don't think it's needed here :)

As for test coverage.. I don't think it's needed because this doesn't really change the behavior of workspace negotiators.

amateescu → created an issue.

Updated the issue summary.

Btw, a quick`n`dirty way to test this MR is to edit a couple of entities in a workspace and manually change WorkspaceViewBuilder.php::$limit to 1 :)

Started a MR with a new approach that builds upon the entity form controller override that was added in 🐛 [PP-1] Prevent content from being deleted when there is an active workspace Postponed . That service is new in 10.3.x, so it doesn't need any deprecation handling :)

Implemented the separate method as discussed above, and it's looks much better!

Regarding \Drupal\Core\Database\Query\TableSortExtender, we can't use it because the data displayed in the table is coming from the individual revision objects, not from the `workspace_association` table.

That works if you have one image gallery with multiple images. What happens if you want two image galleries?

At the storage level, the first gallery can use deltas 0 to 3 (contains 4 items), and the second gallery can use deltas 4 to 6 (contains 3 items). Then at the `TypedData::get()` level we can assign each delta item to its specific component (gallery).

It turns out that the work done in 📌 Add an API for allowing modules to mark their forms as workspace-safe Needs work was pretty much on point and useful for the use cases tackled by this issue.

Some changes in this MR might seem weird, so I'll add some comments on the MR itself to try and explain them directly in a code context.

alexpott → credited amateescu → .

We can do something like this in WSE, but we also need an issue in the Diff module to factor out getting the revision IDs from \Drupal\diff\Form\RevisionOverviewForm::buildForm() into a separate method, because overriding and copying all that code is a bit too much to do in this module.

Once we have that issue (and patch), we need to extend this patch and override \Drupal\diff\Form\RevisionOverviewForm.

@capita that's not possible currently because there's no integration between JSON:API and Workspaces. That's what we're trying to figure out in this issue :)

Fixed both review points :)

Adding the url.path cache context to every page would be a performance regression, and we can avoid it if we use a lazy builder instead for the workspace toolbar tab, just like announcements_feed module does it.

I wasted a lot of time trying to add test coverage, with no success: https://git.drupalcode.org/project/drupal/-/merge_requests/7605/commits . Adding coverage for this should be as easy as adding a few more drupalGet() calls in WorkspaceToolbarIntegrationTest::testWorkspaceSwitch(), but our functional javascript test framework disagrees :/

My proposal is to go with only manual testing here, otherwise the issue will be stalled for who knows how many years...

amateescu → made their first commit to this issue’s fork.

amateescu → made their first commit to this issue’s fork.

We can probably copy what Workspaces is doing: https://git.drupalcode.org/project/drupal/-/blob/11.x/core/modules/works...

📌 Add an API for allowing modules to mark their forms as workspace-safe Needs work will make it possible for contrib/custom modules to declare their forms as workspace-safe.

It's definitely still an issue, I just closed 🐛 After closing workspaces dialog the vertical admin toolbar is displayed on top of workspaces interface Closed: duplicate as a duplicate of this one.

This is a duplicate of 🐛 The position of the toolbar moves above the off-canvas top dialog if a modal dialog is opened Needs work .

Opened a MR for it, and the bug can be easily tested by changing the existing coverage to do menu link operations via the UI, as it should :)

Btw, the problem is not only for sub workspaces, top-level ones have the same issue.

I was 100% sure we had test coverage for this, so I looked into it and turns out it's a problem that can only be reproduced in the UI, and even though WorkspaceMenuLinkContentIntegrationTest is a functional test, it's doing CRUD operations on menu links via API calls :/

Changed the approach to use two interfaces instead of a trait, and updated the issue summary and the change record.

Moved the guide under "Core module" since it was marked stable in https://www.drupal.org/project/drupal/issues/3088643 📌 Mark Workspaces as a stable core module Fixed

I can't reproduce this with latest Drupal 10.2.x and wse, did you have any other wse submodules installed by any chance? I tried with wse_config, wse_lb, wse_menu all installed, and drush pmu wse still worked.

@alexpott the idea with a trait came up specifically for the dynamic nature that some modules might need, like Layout Builder in core :)

The difference is that in HEAD, LB has to call a method in each form class for marking its forms as workspace-safe, while with the trait approach it only has to ensure that its method name matches the one from the "top-level" trait, and Workspaces takes care of calling it in its own form alter.

I don't mind going for the interface approach you suggested, Workspaces itself does that currently and it works great when there's no dynamism involved. Should we wait or ask for other opinions or do you feel (strongly or not) that the interface will be far simpler to implement for the majority use cases?

Workspaces was just marked stable 📌 Mark Workspaces as a stable core module Fixed , so.. 8 years later it's finally time to close this issue! :D

Hiding old patch files.

Even after 10.3, this module will still provide useful features that are not (yet) in core:

- move a piece of content from a workspace to another
- discard changes
- revert a workspace publication
- change menu hierarchies in a workspace with the `wse_menu` submodule

So I think the answer is "yes" :)

I'll probably need to open a 2.x branch that will only be compatible with 10.3+ because there are way too many changes to take into account for previous core versions.

We also need to update that info for our test module, and it's probably time to drop support for 8.7.7 :)

Since @catch was not a fan of the event approach, here's an alternative that would allow modules to declare their forms as workspace-safe even when the workspaces module is not installed.

I added the trait in the Drupal\Core\Form as an initial proposal, but it could also be added somewhere in the system module I guess..

Once Workspaces will be marked as stable, more and more modules will have to integrate with it, so I'm raising the priority of this issue.

amateescu → changed the visibility of the branch 11.x to hidden.

amateescu → changed the visibility of the branch 3208390-add-an-api to hidden.

Posted a few comments on the 11.x MR. Otherwise this looks good to me :)

Oops, I didn't realize we don't need that service anymore. Since the Merger class is not a service and it's also tagged @internal, I only removed the argument from its constructor, but I had to do the whole deprecation dance anyway for WorkspaceOperationFactory because that's an actual service :/

Also added a CR for it: https://www.drupal.org/node/3440755 →

Turns out I was wrong in #2, the problem here is WorkspaceAssociation::onPostPublish() not deleting the associations of sub-workpaces.

Hm.. that's right, I thought 🐛 Sub workspace does not clear Closed: duplicate was about the value of the workspace metadata field, but it's actually about WorkspaceAssociation::onPostPublish() not deleting the associations of sub-workspaces. I'll reopen it :)