Switzerland
Account created on 9 December 2007, over 16 years ago
#

Merge Requests

More

Recent comments

πŸ‡¨πŸ‡­Switzerland Berdir Switzerland

Right now only linkit 7.x with the removed BC layer is D11 compatible, so raising priority, as without this, this will fail with linkit 7.x/D11.

πŸ‡¨πŸ‡­Switzerland Berdir Switzerland

Did that in πŸ“Œ Automated Drupal 11 compatibility fixes for replicate Fixed . These deprecations had nothing to do with PHP 8.2.

πŸ‡¨πŸ‡­Switzerland Berdir Switzerland

Merged. Messed up a bit and got the bot MR first, had to revert and rebase.

πŸ‡¨πŸ‡­Switzerland Berdir Switzerland

Berdir β†’ made their first commit to this issue’s fork.

πŸ‡¨πŸ‡­Switzerland Berdir Switzerland

I was a bit concerned about the hal related test fails I saw locally, but it's caused by hal, not default_content and it's fixed by ✨ compatible with Drupal 11 Needs review

πŸ‡¨πŸ‡­Switzerland Berdir Switzerland

I meant regular config, that's what we've done then. A post update to enable the setting on existing sites, and later on another to remove it again. Thinking about update database dumps and similar, I believe that would be easier to deal with than having to remove it again from those.

πŸ‡¨πŸ‡­Switzerland Berdir Switzerland

Would it be easier to just add a setting instead of a whole module? Similar to how hal had the BC type cast setting in D8/D9.

πŸ‡¨πŸ‡­Switzerland Berdir Switzerland

This is almost ready now.

only failing tests on D11 are rdf, which isn't compatible yet.

Unfortunately, requiring tour from seems to break testing on 10.1. Easiest option might be to just wait a few more days, when 10.3 is out then it will be testing 10.2 and 10.3 instead.

πŸ‡¨πŸ‡­Switzerland Berdir Switzerland

I'm very confused about this change. See my MR comment, js might not be used much directly, but the js-show and js-hide classes have 10+ usages in core alone and the proposed change completely breaks the functionality of that.

πŸ‡¨πŸ‡­Switzerland Berdir Switzerland

rebased on top of πŸ“Œ Add gitlab template to run the CI RTBC , enabled next major testing, and started fixing the deprecated $supportedInterfaceOrClass definitions to getSupportedTypes().

πŸ‡¨πŸ‡­Switzerland Berdir Switzerland

There is no such code in this project, you are likely using a patch from ✨ Add a Normalizer and Denormalizer to support Layout Builder Needs work

πŸ‡¨πŸ‡­Switzerland Berdir Switzerland

Confirmed that next minor is now green and removed that again. Also enabled concurrent testing, which reduces test execution time from 30min to 3-4min.

πŸ‡¨πŸ‡­Switzerland Berdir Switzerland

Starting with 10.2, the revision log is exported as an empty string. I'm actually not sure why and didn't investigate. Still works in 10.1, so I assume it's a core change. maybe create a follow-up so we can unblock this and D11 testing?

Starting with 10.3, the user registration behavior changes due to πŸ› User created via /user/register?_format=json get blocked Fixed .

Both tests fixed with version checks.

See also my note above on previous minor testing, that is IMHO useful, more so than next minor, once it's green I'll remove that from the MR.

πŸ‡¨πŸ‡­Switzerland Berdir Switzerland

Berdir β†’ made their first commit to this issue’s fork.

πŸ‡¨πŸ‡­Switzerland Berdir Switzerland

The project page explains this.

πŸ‡¨πŸ‡­Switzerland Berdir Switzerland

I think before we merge this we should verify the impact of this with some of the more complex projects like paragraphs to see if this impacts test execution time, does it make it slower/faster/more reliable? The related core issue is now in, but only on 10.3+. So that would allow to compare with/without that with the phpunit vs phpunit next minor job.

Should be doable with a paragraphs MR that overrides the phpunit script changes 32 to 8.

However, a dynamic calculation isn't supported by the GitLab CI API at this time. This is not only because they don't support bash scripts in variable declarations, it's more so because the variables are determined before the runner kicks in, i.e. at that point we don't seem to have any detailed information about the environment yet.

FWIW, there are plenty of examples in https://git.drupalcode.org/project/gitlab_templates/-/raw/main/includes/... that dynamically set variables, for example for BC. Basically by having two variables, one to set it manually, the other as fallback that's calculated. Can't be a variable, but it can be part of the script commands. Not sure yet if needed, but it is possible.

πŸ‡¨πŸ‡­Switzerland Berdir Switzerland

lenient can't solve the conflict that devel currently has: https://gitlab.com/drupalspoons/devel/-/issues/527, temporarily removed from require-dev.

also made all submodules compatible and fixed an invalid namespace.

We have actual test fails now to work with.

πŸ‡¨πŸ‡­Switzerland Berdir Switzerland

Added lenient, also pushed to separate branch to not work on top of the bot branch.

πŸ‡¨πŸ‡­Switzerland Berdir Switzerland

Berdir β†’ made their first commit to this issue’s fork.

πŸ‡¨πŸ‡­Switzerland Berdir Switzerland

Is this really that useful with the category selection mode? This opens up the ability to use kinds of blocks, including special ones like title/messages/content that are usually not desired or could even result in errors or recursion.

The category selection mode on other one hand allows you to allow any block_content entity if you want to, or all views or any any combination of that.

The MR looks like a pretty incomplete and invalid port of an older patch, the settings form changed a lot. there's no plugin_ids anymore on that level ever, and now it's making the selection *mode* optional, not the plugins.

I don't know if this really still does work, and I won't commit it without tests.

A better approach for this might be to build on the selection mode plugin that explicitly allows everything

πŸ‡¨πŸ‡­Switzerland Berdir Switzerland

All I'm asking is creating a separate issue with a new MR, nothing has been sent down any drain: πŸ“Œ Update links in documentation Needs work , still needs work though.

πŸ‡¨πŸ‡­Switzerland Berdir Switzerland

Needs work to remove the versions completely, should default to the current core version then.

πŸ‡¨πŸ‡­Switzerland Berdir Switzerland

Berdir β†’ created an issue.

πŸ‡¨πŸ‡­Switzerland Berdir Switzerland

Needed some workarounds for config schema changes in 10.3 and add another submit changes in 10.2, now all tests are passing from previous major to next major, merged.

πŸ‡¨πŸ‡­Switzerland Berdir Switzerland

Berdir β†’ made their first commit to this issue’s fork.

πŸ‡¨πŸ‡­Switzerland Berdir Switzerland

Yes, that patch is wrong, we can't just not run the logic. What should be done, should have been done in the original issue is to fallback to the \Drupal::entityTypeManager() if that's not provided.

πŸ‡¨πŸ‡­Switzerland Berdir Switzerland

I think it make more sense as a separate issue, that's not actually necessary for D11 compatibility. Also, I think there's a way to remove the version from those links and it will pick the default.

πŸ‡¨πŸ‡­Switzerland Berdir Switzerland

Berdir β†’ made their first commit to this issue’s fork.

πŸ‡¨πŸ‡­Switzerland Berdir Switzerland

Nothing in core verifies this, so it's not possible to have failing tests. Merged.

πŸ‡¨πŸ‡­Switzerland Berdir Switzerland

Merged.

πŸ’¬ | Redis | Valkey Support
πŸ‡¨πŸ‡­Switzerland Berdir Switzerland

As long as Valkey remains API compatible with original Redis, there's no reason why you shouldn't be able to use it.

πŸ‡¨πŸ‡­Switzerland Berdir Switzerland

@jurgenhaas: Both merge requests yes, but if you click on them or check the hidden branches on the issue summary you can see that both branches were opened for the same branch, project-update-bot-only. There's no guarantee with how bad the bot got confused here, but IMHO, using 3433816-automated-drupal-11 is as safe as any other branch on this fork.

πŸ‡¨πŸ‡­Switzerland Berdir Switzerland

@apaderno: Maybe it's meant to be a Haiku? ;) ( don't actually have any clue about how Haikus work)

πŸ‡¨πŸ‡­Switzerland Berdir Switzerland

This is overwhelmingly large, has some bad changes and needs to be done as a merge request.

Many of these changes probably already have existing dedicated issues, or multiple, it likely makes more sense to focus on these, this will likely never get committed in this form.

  1. +++ b/entity_browser.api.php
    @@ -3,15 +3,14 @@
     /**
    - * Alter the information provided in \Drupal\entity_browser\Annotation\EntityBrowserDisplay.
    + * Alter the information provided in.
    + *
    + * \Drupal\entity_browser\Annotation\EntityBrowserDisplay.
      *
      * @param array $displays
      *   The array of display plugins, keyed on the machine-readable name.
    @@ -21,7 +20,9 @@ function hook_entity_browser_display_info_alter(array &$displays) {
    

    this doesn't make sense, coder might be happy, but this isn't valid documentation.

  2. +++ b/modules/entity_form/entity_browser_entity_form.module
    @@ -155,7 +155,8 @@ function entity_browser_entity_form_reference_form_submit(array $reference_form,
       $form_state->setValueForElement($reference_form['entity_browser']['entity_ids'], '');
       $input = &$form_state->getUserInput();
    -  NestedArray::unsetValue($input, array_merge($reference_form['#parents'], ['entity_browser', 'entity_ids']));
    +  NestedArray::unsetValue($input, array_merge($reference_form['#parents'],
    +  ['entity_browser', 'entity_ids']));
     }
     
    

    this is not more readable and this coder rule is controversial and not enabled by core. We could copy the core phpcs rules, see πŸ“Œ Fix the issues reported by phpcs Needs review

  3. +++ b/modules/entity_form/src/Plugin/EntityBrowser/Widget/EntityForm.php
    @@ -152,8 +152,20 @@ class EntityForm extends WidgetBase {
         $parents = ['table', $this->uuid(), 'form'];
         $entity_type = $form_state->hasValue(array_merge($parents, ['entity_type'])) ? $form_state->getValue(array_merge($parents, ['entity_type'])) : $this->configuration['entity_type'];
    -    $bundle = $form_state->hasValue(array_merge($parents, ['bundle', 'select'])) ? $form_state->getValue(array_merge($parents, ['bundle', 'select'])) : $this->configuration['bundle'];
    -    $form_mode = $form_state->hasValue(array_merge($parents, ['form_mode', 'form_select'])) ? $form_state->hasValue(array_merge($parents, ['form_mode', 'form_select'])) : $this->configuration['form_mode'];
    +    $bundle = $form_state->hasValue(array_merge($parents, [
    +      'bundle',
    +      'select',
    +    ])) ? $form_state->getValue(array_merge($parents, [
    +      'bundle',
    +      'select',
    +    ])) : $this->configuration['bundle'];
    +    $form_mode = $form_state->hasValue(array_merge($parents, [
    +      'form_mode',
    +      'form_select',
    +    ])) ? $form_state->hasValue(array_merge($parents, [
    +      'form_mode',
    +      'form_select',
    +    ])) : $this->configuration['form_mode'];
     
         $definitions = $this->entityTypeManager->getDefinitions();
    

    same, this doesn't make it more readable. getValue() has a default argument, not sure why this code isn't using that.

  4. +++ b/src/Ajax/ValueUpdatedCommand.php
    @@ -10,11 +10,11 @@ use Drupal\Core\Ajax\CommandInterface;
     class ValueUpdatedCommand implements CommandInterface {
     
       /**
    -   * The ID for the details element.
    +   * Details element ID.
        *
        * @var string
        */
    -  protected $details_id;
    +  protected $detailsIds;
     
    

    This can be considered an API change.

  5. +++ b/src/Events/Events.php
    @@ -8,33 +8,36 @@ namespace Drupal\entity_browser\Events;
       const SELECTED = 'entity_browser.selected';
     
       /**
    -   * The DONE event occurs when selection process is done. While it can be emitted
    -   * by any part of the system that will usually be done by selection display plugin.
    +   * The DONE event occurs when selection process is done.
    +   *
    +   * While it can be emitted by any part of the system.
    +   *
    +   * That will usually be done by selection display plugin.
        *
    

    same here, this blindly makes phpstan happy, but the result is no longer English.

πŸ‡¨πŸ‡­Switzerland Berdir Switzerland

> but there's nothing we can do about that here.

So, that's not entirely true. What we can do is add a conflict with the broken version(s), but not sure we should do that before we know which version the fix will be in. Still, it could be useful to prepare the MR for that and hope the next patch releases will have the fix.

πŸ‡¨πŸ‡­Switzerland Berdir Switzerland

There are many new deprecations in 10.3. They are added continuously to the next minor version. You can't expect to be deprecation free for the next minor version, or even the current version if you want to keep support for earlier versions. Sometimes there are workarounds, with if/else constructs, DeprecationHelper and what not, but for some deprecations, that just not possible or only with a lot of effort (you could probably do some magic at config install time if you find a hook/event/wrapped service that's early enough)

That's not a problem, that doesn't need to be fixed. There are no expectations to be deprecation-free at any point, it's an early warning system that something will change in the future, nothing is broken yet. There is a reason that deprecations are off by default in Gitlab CI.

Trying to keep next* test runs deprecation free is just a continuous race against Drupal core development. The only thing you can reliably go for is to be deprecation free on the oldest version you provide support for.

πŸ‡¨πŸ‡­Switzerland Berdir Switzerland

Committed, but note that modules relying on node parameter being a node object are wrong and must check for this.

πŸ‡¨πŸ‡­Switzerland Berdir Switzerland

Merged, thanks.

πŸ‡¨πŸ‡­Switzerland Berdir Switzerland

Berdir β†’ made their first commit to this issue’s fork.

πŸ‡¨πŸ‡­Switzerland Berdir Switzerland

Yes, this should be fixed now.

πŸ‡¨πŸ‡­Switzerland Berdir Switzerland

previous/current/next minor testing is the best I could manage, not sure what's up with the failing test on previous and not going to spend more time on that. Merged.

πŸ‡¨πŸ‡­Switzerland Berdir Switzerland

Berdir β†’ created an issue.

πŸ‡¨πŸ‡­Switzerland Berdir Switzerland

That backtrace clearly shows that this is the simpleSAMLphp bootstrap, and not Drupal. I can see that it's being reported in symfony and I guess beig fixed there, but if not, such an error needs to be reported in https://github.com/simplesamlphp/simplesamlphp, not here. Keeping this open as a support request for for visibility, but there's nothing we can do about that here.

πŸ‡¨πŸ‡­Switzerland Berdir Switzerland

The merge request seems backwards and changes things back to version 1?

πŸ‡¨πŸ‡­Switzerland Berdir Switzerland

It makes more sense to do this as part of πŸ“Œ Automated Drupal 11 compatibility fixes for default_content Needs review because it's needed for D11 compatibility and we can't test otherwise.

On the other hand, this breaks compatibility with version 10.1 and earlier, at least for running tests, so it's not something to do lightly.

πŸ‡¨πŸ‡­Switzerland Berdir Switzerland

Same as https://git.drupalcode.org/project/paragraphs/-/merge_requests/115/diffs..., you could try to get the ball rolling on D11 testing by using lenient for all test dependencies that aren't compatible yet.

πŸ‡¨πŸ‡­Switzerland Berdir Switzerland

D11 is not going to work with a phpstan deprecation like this:


Line tests/src/Kernel/TranslationNormalizerTest.php
------ -----------------------------------------------------------------
19 Usage of deprecated trait
Drupal\Tests\field\Traits\EntityReferenceTestTrait in class
Drupal\Tests\default_content\Kernel\TranslationormalizerTest:
in drupal:10.2.0 and is removed from drupal:11.0.0. Use
\Drupal\Tests\field\Traits\EntityReferenceFieldCreationTrait
instead.

Unfortunately, fixing this means we can no longer test on 10.1 and lower.

One way to verify D11 test are actually working is to use a lenient configuration. like this: https://git.drupalcode.org/project/paragraphs/-/merge_requests/115/diffs..., then it should be able to install and only the tests that actually require those additional modules should fail.

πŸ‡¨πŸ‡­Switzerland Berdir Switzerland

A merge request with a .gitlab-ci.yml that tests previous/next minor/major is necessary to verify this works and tests pass.

πŸ‡¨πŸ‡­Switzerland Berdir Switzerland

The file_validate() deprecation is not pretty, in some cases I'm not sure where the upload validators could come from, so I did my best to keep BC for it. Might not more changes when entity browser itself is updated too, we'll see.

All tests pass locally but this is blocked on entity_browser getting compatible first.

πŸ‡¨πŸ‡­Switzerland Berdir Switzerland

Berdir β†’ created an issue.

πŸ‡¨πŸ‡­Switzerland Berdir Switzerland

The answer to the puzzle is that wikimedia/composer-merge-plugin is deprecated, very bad for composer performance and should not be used anymore. The file is just there for backwards compatibility.

πŸ‡¨πŸ‡­Switzerland Berdir Switzerland

Merged.

πŸ‡¨πŸ‡­Switzerland Berdir Switzerland

This should pass now. Made minimal fixes to the revision route provider only, but most of that code can be removed/deprecated now, but that's for a separate issue, this already needs to make enough changes to data providers.

Either way need to require 10.1 due to the revision controller changes.

πŸ‡¨πŸ‡­Switzerland Berdir Switzerland
πŸ‡¨πŸ‡­Switzerland Berdir Switzerland

Berdir β†’ created an issue.

πŸ‡¨πŸ‡­Switzerland Berdir Switzerland

This isn't the same branch than the bot used, that one is named "project-update-bot-only", the one that has automated in the name is just named because it's the default branch created by drupal.org for the issue fork based on the issue title name. That one should not get overwritten.

πŸ‡¨πŸ‡­Switzerland Berdir Switzerland

Changing back to needs work for now.

There are two changes now here.

1) The original change to use the published entity key. Without also changing \Drupal\content_translation\ContentTranslationHandler::hasPublishedStatus(), this does nothing. Any entity that does not have a status field will automatically get the content_translation_status field, and it will never look for status or a published field. That also means this can't be tested with a kernel or functional test since that code path doesn't exist. A unit test would be able to do it, but at that point we're faking up a scenario that's not real. Therefore it is IMHO not useful to make this change in isolation.

2) The other change is using EntityPublishedInterface, which is a performance optimization. Nothing is broken, it's just isn't as fast as it could be. So we can't have a failing kernel or functional test again. We could wire up a unit test and explicitly assert that it's calling isPublished() when the passed in object implements that interface, but I'm not sure if it's worth doing that.

At the same time, 2) isn't as good as it could be without having as many cases as possible go through that check, so getting that in on its own isn't *that* useful.

> one about menu_link_content

That can't be a separate follow-up either, because it's directly tied to the hasPublicationStatus() logic. If we change that then menu_link_content falls apart. We could customize its handler and wrapper classes to not do that, but we still will break contrib and cutom entity types in the same way, so we need a solution for that anyway.

These content_translation issues are really hard to untangle...

πŸ‡¨πŸ‡­Switzerland Berdir Switzerland

I recovered my local old branch with all the work that was done here, rebased on 8.x-1.x, pushed to https://git.drupalcode.org/project/paragraphs/-/merge_requests/115. Closed all other merge requests.

πŸ‡¨πŸ‡­Switzerland Berdir Switzerland

Berdir β†’ changed the visibility of the branch 3433816-manual-drupal-11 to hidden.

πŸ‡¨πŸ‡­Switzerland Berdir Switzerland

That was just a joke, I do hope the update bot is not sentient enough to punish us ;)

but yes, we should push the changes to a separate branch (in the same fork) maybe the one that got created by default already, and continue there. not expecting much more from the bot here anyway.

also, I had already extracted all the changes to fix 10.2/10.3 tests into πŸ“Œ Drupal 10.2/10.3 test fixes Fixed , the blocker now is that migration stuff, that sounds like it might require a major rewrite.

Honestly tempted to just rip that all out and push it in a separate contrib project and let someone else maintain it.

πŸ‡¨πŸ‡­Switzerland Berdir Switzerland

we shouldn't have worked on that anyway, it did warn us, I'll reupload a local one, but there might also be a tagged backup.

(I would expect it would have just deleted our changes, not use a 4 year old one, but I guess it wanted to make a point or something ;))

πŸ‡¨πŸ‡­Switzerland Berdir Switzerland

Leaving at this status, but per #15, I don't think this is really done.

This is only half the picture, the other half is \Drupal\content_translation\ContentTranslationHandler::hasPublishedStatus(), which still has the hardcoded status.

That means the only effective change here is that entities that *do* have a status field will now use isPublished(), which makes this check likely a bit faster (didn't profile yet).

But entities that don't have a status field but have a published entity like like MenuLinkContent are not yet affected at all. As written, changing them would result in an immediate field structure change that is hard to deal with.

πŸ‡¨πŸ‡­Switzerland Berdir Switzerland

Berdir β†’ made their first commit to this issue’s fork.

πŸ‡¨πŸ‡­Switzerland Berdir Switzerland

Closing as duplicate of πŸ› a Drupal 11 dev line is needed for CROP API. RTBC

πŸ‡¨πŸ‡­Switzerland Berdir Switzerland

per https://git.drupalcode.org/issue/entity-3427503/-/jobs/1752217, there are still \Drupal:: calls in EntityViewsData, if we're doing this then lets do it for all cases.

Several of the calls there can be removed now because the parent class has that property. For BC, lets keep the method, just remove the fallback. the entity type info property doesn't seem to be in the base class despite having the same todo though.

πŸ‡¨πŸ‡­Switzerland Berdir Switzerland

Must be done as a merge request now so that we can verify with CI.

πŸ‡¨πŸ‡­Switzerland Berdir Switzerland

Makes sense, merged.

πŸ‡¨πŸ‡­Switzerland Berdir Switzerland

Berdir β†’ made their first commit to this issue’s fork.

πŸ‡¨πŸ‡­Switzerland Berdir Switzerland

Not sure what's up with previous major, disabled for now. Merged.

πŸ‡¨πŸ‡­Switzerland Berdir Switzerland

Berdir β†’ made their first commit to this issue’s fork.

πŸ‡¨πŸ‡­Switzerland Berdir Switzerland

Tests were fixed in πŸ“Œ Add Gitlab CI Fixed

πŸ‡¨πŸ‡­Switzerland Berdir Switzerland

Changed this in πŸ“Œ Add Gitlab CI Fixed as tests were now broken in D10.

πŸ‡¨πŸ‡­Switzerland Berdir Switzerland

Should be done as a merge request, space missing after ,

πŸ‡¨πŸ‡­Switzerland Berdir Switzerland

Merged.

πŸ‡¨πŸ‡­Switzerland Berdir Switzerland

Berdir β†’ made their first commit to this issue’s fork.

πŸ‡¨πŸ‡­Switzerland Berdir Switzerland

And I commented in that issue again that I consider this to be worse code and am not interested in following this rule.

πŸ‡¨πŸ‡­Switzerland Berdir Switzerland

Thanks, merged.

πŸ‡¨πŸ‡­Switzerland Berdir Switzerland

Berdir β†’ made their first commit to this issue’s fork.

πŸ‡¨πŸ‡­Switzerland Berdir Switzerland
πŸ‡¨πŸ‡­Switzerland Berdir Switzerland

Berdir β†’ made their first commit to this issue’s fork.

πŸ‡¨πŸ‡­Switzerland Berdir Switzerland

Berdir β†’ made their first commit to this issue’s fork.

πŸ‡¨πŸ‡­Switzerland Berdir Switzerland

Fixed and merged.

πŸ‡¨πŸ‡­Switzerland Berdir Switzerland

This is in the bynder php sdk, not this module: https://github.com/Bynder/bynder-php-sdk/issues/56

πŸ‡¨πŸ‡­Switzerland Berdir Switzerland

Also, the problem with this approach is that we lose the check on field translation that \Drupal\content_translation\ContentTranslationMetadataWrapper::setFieldOnlyIfTranslatable() does.

πŸ‡¨πŸ‡­Switzerland Berdir Switzerland

#20/#21 doesn't make sense, setOwner() isn't return a field that can be set. if the entity implements the necessary interface. Instead it needs to use that field if it exists, and otherwise call setOwner(). See also πŸ“Œ Content Translation should use EntityPublishedInterface or the 'published' entity key to determine if it needs to add its 'content_translation_status' field Needs review on BC concerns, this will have an impact on defined field storages.

πŸ‡¨πŸ‡­Switzerland Berdir Switzerland

I stumbled over this also from a performance perspective.

isPublished() is called a lot from content_translation_language_fallback_candidates_entity_view_alter() if you are displaying many entities, for example nodes in a view or embedded things like paragraphs or also menu link content entities, this is currently quite slow.

We have EntityPublishedInterface now, which is optimized to use the entity key value system which allows to return this value without instantiating a field object.

Created a merge request from this and extend the code to also check for EntityPublishedInterface. Unfortunately, this doesn't actually work for menu_link_content, the biggest offender for performance in our case, exactly because of the published/status situation. Just changing this class to respect that isn't enough, we also would need to change \Drupal\content_translation\ContentTranslationHandler::hasPublishedStatus(), but that's where things get complicated. If we just fix that, then content_translation will switch over to the enabled field and stop using its own field which might have data in it. I don't really know how to handle that. We would probably need to write an update function, loop over all enabled entity types that have no status field but have a published entity key and then migrate the data over, assuming translatability of the field is set correctly.

Note that the wrapper currently also blindly assumes that the entities it wraps have a getOwner() and a getChangedTime() method, without checking for the respective interfaces. There's also πŸ› Content Translation does not use EntityOwnerInterface properly Needs work which touches the owner topic.

Changing this to a task because I don't think it's really a bug, the wrapper can be subclassed, if it doesn't work for you you can customize it, it's a performance optimization/code generalization so it works for more cases out of the box. On the plus side, I think it doesn't really need tests for the current changes. It will need tests if we go all the way for the menu_link_content implications for example.

πŸ‡¨πŸ‡­Switzerland Berdir Switzerland

Back to green!

πŸ‡¨πŸ‡­Switzerland Berdir Switzerland

It would be really neat to add a linkit integration test as well, we can posibly copy paste something from linkit as a starting point, that would help catch integration issues early and also help with the current phpstan issue about the unknown class when we add a require-dev for linkit.

Production build 0.69.0 2024