Account created on 21 February 2010, over 15 years ago
#

Merge Requests

More

Recent comments

🇷🇴Romania amateescu

Well.. that's very much internal to the content entity storage and I don't really see a reason for outside code/storage overrides to do something with it.

The alternative would be to add a new method specifically for clearing it, but I don't think that's preferable to the current approach from the MR. Core committers can decide I guess :)

🇷🇴Romania amateescu

Okay, sorry about that :)

🇷🇴Romania amateescu

Right, but this request is for transferring the project ownership, that's why I only took @rötzi into account.

🇷🇴Romania amateescu

Still interested :)

🇷🇴Romania amateescu

I've tried to reproduce this quickly by putting this code in a template, and the output was correct, trashed nodes were not displayed:

{% if drupal_view_result('frontpage', 'page_1') is not empty %}
  <h2 class="section-heading">Publications</h2>
  {{ drupal_view('frontpage', 'page_1') }}
{% endif %}

Does your view have a filter on the deleted field? Because that's the only way to bypass Trash's Views query alter, see https://git.drupalcode.org/project/trash/-/blob/3.x/src/ViewsQueryAlter....

🇷🇴Romania amateescu

You can create paragraph entities in a workspace if you have WSE installed or this patch, but they're not tracked by that workspace :) The CRUD part mostly means "allow any operation for this entity type but don't track it".

🇷🇴Romania amateescu

I've read through the latest comments and I agree that having access to enable/disable queues through the update permission is problematic.

However, I think it's an advanced enough use-case that we can use the administer entityqueue permission instead of adding a new one. Updated the MR to do that and also fixed a few other things.

Merged into 8.x-1.x, thanks for all the work on this!

🇷🇴Romania amateescu

I tried to reproduce this using the steps in the issue summary and I couldn't get the view to display anything after adding the second contextual filter. Can you please update the steps with actual values? Even node title and values for the body field would help to ensure that I'm testing your specific scenario.

And you can leave out the parts about adding the modules, installing the site, etc. as they're not necessary :)

🇷🇴Romania amateescu

Or XB would need to pick up the message from some generic source that Trash altered :)

That's exactly what needs to happen :) Trash alters the description message of \Drupal\Core\Entity\EntityDeleteForm (actually coming from its parent: \Drupal\Core\Entity\EntityConfirmFormBase::getDescription()), and XB's xb_page entity type declares that it's using that form, but the message from the screenshot seems to be provided by a JS file instead: ui/src/components/navigation/Navigation.tsx

🇷🇴Romania amateescu

Committed to 3.x, thanks for reporting it! I'll tag a new release as well :)

🇷🇴Romania amateescu

We still need to handle deprecated routes somehow, per #5 and following discussion :)

🇷🇴Romania amateescu

Thanks for opening this (very detailed!) bug report :)

Automatically purging nodes would require a batch operation for that form, so I'd prefer to let the system work as intended and prevent the deletion of that content type instead.

🇷🇴Romania amateescu

Thanks for opening this issue. While I agree that this case needs to be handled better, I don't agree with the AI that generated the solution (and the issue summary by the looks of it), and I think we need to throw a helpful exception in order to make the developer aware of the problem.

🇷🇴Romania amateescu

Yup, that's the correct way to bypass trash and purge entities directly :)

🇷🇴Romania amateescu

Bulk purging (and restoring) will be implemented in Add bulk restore function Active :)

🇷🇴Romania amateescu

Oops, I wasn't looking at the changes tab. Not sure what happened there but I couldn't do anything with that branch so I opened a new MR instead.

🇷🇴Romania amateescu

@smustgrave, not sure why you think so, the MR was applying just fine. Merged the latest 11.x though just in case.

🇷🇴Romania amateescu

Merged into 3.x, thanks!

🇷🇴Romania amateescu

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

🇷🇴Romania amateescu

This was recently fixed here. You can use the dev version until a new release is tagged :)

🇷🇴Romania amateescu

If it doesn't match, we can call RouteProvider::getRouteByName() on each of the passed in routes, this will trigger the deprecation we need.

@catch, the issue is that RouteMatch::isRouteName() will _not_ match the current route name in most (almost all) cases, so we'll be calling ::getRouteByName() all the time. Maybe it's easier to explain with an example:

Let's say we have a few different modules, and each one checks for a route, e.g. ::isRouteName('module1_controller'), ::isRouteName('module2_form'), etc.

Now, imagine that those checks happen on every request, wouldn't this mean that we'll be "attaching" the knowledge of those routes to every page in the context of the route preloader?

🇷🇴Romania amateescu

I don't think that subclassing the version history controller is a good solution for this issue.

The Diff module is a great example. From the perspective of another contrib module which also needs to alter the revision overview, and in order to support both core's controller and Diff's override, it has to subclass both of them. A third one with similar requirements would bring chaos :)

Adding a hook as in the current MR is one way to do it, but I think converting that controller to a form would be more helpful in the long run, especially since Diff (which is a very popular module) already does that.

🇷🇴Romania amateescu

@catch, yep, the space is added for a bit of padding, and it's handled correctly on RTL languages.

🇷🇴Romania amateescu

Updated the test to use key-value instead of state.

@catch, copying FormController::getContentResult() wouldn't help us in any way, it's HtmlEntityFormController::getFormObject() that builds the form, and the MR is actually doing what you're suggesting, it copies the relevant parts from it into WorkspacesHtmlEntityFormController::getContentResult() :)

🇷🇴Romania amateescu

Yay, hook ordering actually works now :) Reverted that change and made EntityOperations::entityFormAlter() run first. Back to RTBC because there's no functional change to the MR and the current code has already been reviewed before.

🇷🇴Romania amateescu

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

🇷🇴Romania amateescu

This is still relevant and not implemented yet :)

🇷🇴Romania amateescu

Decided to set the initial workflow states with database queries. If any other viable entity storage gets written, we can worry about it then :)

🇷🇴Romania amateescu

Posted a few comments on the MR.

🇷🇴Romania amateescu

Try to clear any data you have in the workspace column of the paragraphs_item_revision table before running that update function. Something like this:

drush php:eval "\Drupal::database()->update('paragraphs_item_revision')->fields(['workspace' => NULL])->execute();"
🇷🇴Romania amateescu

You should run this update function using Drush or whatever other method you have available: wse_update_10001().

🇷🇴Romania amateescu

This looks like a valid feature request still.

🇷🇴Romania amateescu

@adamzimmermann

It feels weird to need a contrib module for core functionality to work though.

That's because core moves at a slower pace, and production sites needed a lot of various fixes for Workspaces that were lingering for years in the core queue for various reasons. Workspaces Extra had to incorporate all these little fixes to prevent developers from using a huge list of patches into their projects.

Opened a MR with a fix and test coverage for this issue.

🇷🇴Romania amateescu

That code is now in \Drupal\Core\Entity\Sql\SqlContentEntityStorage::loadFromDedicatedTables(), and it has the same problem :)

🇷🇴Romania amateescu

Addressed all the feedback so far, but there's one thing left.. the main reason for this API: checking whether the passed-in route name is deprecated.

The problem is that the only way to do that currently is \Drupal\Core\Routing\RouteProvider::getRouteAliases(), which would mean an uncached db query for every call to the new RouteMatch::isRouteName() method. I think we need a way to retrieve and cache (persistent and static) all aliased/deprecated routes somewhere in the route provider. Setting back to NW to get feedback on that.

🇷🇴Romania amateescu

The test failures are not caused by this MR, see 🐛 Fix failing PHPunit tests Active .

🇷🇴Romania amateescu

Glad you were able to solve it.

🇷🇴Romania amateescu

Can you clear caches before running the database upgrade?

🇷🇴Romania amateescu

Did you run the database updates?

🇷🇴Romania amateescu

Whoops :) Merged into 2.0.x, thanks!

Production build 0.71.5 2024