Account created on 23 December 2007, almost 17 years ago
#

Merge Requests

More

Recent comments

🇧🇪Belgium rp7

I took a stab at this. Check the issue fork.
Next to the dynamic entity autocomplete form element, it also provides a field widget that uses this form element.
Check the attached recording.

🇧🇪Belgium rp7

Re-rolled against latest 2.x + fixed an issue when copy/paste something in the autocomplete field (this issue was introduced in patch in #31).

🇧🇪Belgium rp7

Re-rolled against 2.0.0.

I now see a search_published_contents_only setting was introduced. IMO this should make use of the event this ticket is introducing. Didn't have the time for this yet (nor for creating a test).

🇧🇪Belgium rp7

+1 for this. My config is riddled with "empty" config like this.

People looking for a temporary workaround, this one seems to work for me:

/**
 * Implements hook_field_widget_third_party_settings_form().
 */
function mymodule_ux_field_widget_third_party_settings_form(WidgetInterface $plugin, FieldDefinitionInterface $field_definition, $form_mode, array $form, FormStateInterface $form_state) {
    $element = [];

    // Add a "nested_hide_level" checkbox to the settings form.
    $element['mysetting'] = [
      '#type' => 'checkbox',
      '#title' => $this->t('My setting'),
      '#default_value' => $plugin->getThirdPartySetting('mymodule', 'mysetting'),
      '#required' => FALSE,
      '#element_validate' => ['_mymodule_validate_mysetting'],
    ];

    return $element;
}

/**
 * Validates mysetting.
 */
function _mymodule_validate_mysetting(&$element, FormStateInterface $form_state, &$complete_form) {
  if (empty($element['#value'])) {
    $form_state->unsetValue(array_slice($element['#parents'], 0, -1));
  }
}
🇧🇪Belgium rp7

Argh, sorry. But I think I cheered a little too early.
It looks like updates are now correctly imported, but a deleted mapping is not.
So if I delete a mapping for a field from the config file, import it & perform drush cex -y - it's added again.
Any idea?

🇧🇪Belgium rp7

Sorry for the late reply, but it looks like it works. Thanks!

🇧🇪Belgium rp7

Sorry for posting in this old issue, but I was wondering: could it be that we forgot to update the return type of \Drupal\Core\Entity\EntityChangedInterface::getChangedTime() ?

This is still

/**
  * Gets the timestamp of the last entity change for the current translation.
  *
  * @return int
  *   The timestamp of the last entity save operation.
  */
public function getChangedTime();

While \Drupal\Core\Entity\EntityChangedTrait::getChangedTime() is now documented to be able to return NULL values:

  /**
   * Gets the timestamp of the last entity change for the current translation.
   *
   * @return int|null
   *   The timestamp of the last entity save operation. Some entities allow a
   *   NULL value indicating the changed time is unknown.
   */
  public function getChangedTime() {
    $value = $this->get('changed')->value;
    return isset($value) ? (int) $value : NULL;
  }
🇧🇪Belgium rp7

I've found the code that's causing this, although it's not clear to me why this code is there in the first place. Removing it fixes the issue & I don't immediately see something that breaks without this code.

Patch attached removes the code. Not a real solution, more of a (temporary) quick-fix to work around this issue.

🇧🇪Belgium rp7

@guignonv
I think that was idd something that creeped into the MR when resolving conflicts. Reverted that change.

🇧🇪Belgium rp7

Fork works now. I'm hiding the patch. Latest changes can be seen in the MR.

🇧🇪Belgium rp7

Uploading my current work as a patch for now, since this issue's fork returns a 404 page in GitLab (https://git.drupalcode.org/issue/external_entities-3475248/-/import) - hopefully that fixes itself soon.

This patch removes the need to use the inline entity form module completely for annotations. The external entity type's annotation fields now also appear on the form display configuration page, where you can configure the position and widgets of these fields as if they were regular fields.

This allows us to remove all the quirks/workarounds in this module to be compatible with inline entity form. I see no reason anymore why one would use inline entity forms for annotations.

The patch currently also picks up the feature requested @ Filter the annotation field Needs work , including an automatic upgrade path for this.

What I currently still need to test/work on is how this works in combination with translations (e.g. the work being done in Make external entities translatable Needs work ).

🇧🇪Belgium rp7

Hmm, that's odd.

I started a project from scratch and quickly created an external entity type (through the UI) called my_external_entity_type and the validation error is also present for this one.

I attached a screenshot + the external entity type's config export - hopefully you can reproduce it as well.

🇧🇪Belgium rp7

@guignonv Thanks!
I'm spotting an issue though when upgrading from 2.x:

>  [error]  Cannot add field 'xntt_rest_queries.ephash': field already exists. 
>  [error]  Update failed: external_entities_update_93015 

Probably because when I'm upgrading my project, external_entities_update_9305() is also being run (which installs the xntt_rest_queries table).

See patch attached. Maybe we should add something like this?

🇧🇪Belgium rp7

Thx! Maybe a new issue would be useful for such a clean-up.

I think you forgot to merge this, though (:

🇧🇪Belgium rp7

I tried switching the issue fork branch to 3.x but can't get it to work. Might be because the initial ticket version was 2.x?
Anyway, I'm closing this issue & re-creating it for 3.x @ Make external entities translatable Needs work . Sorry for the mess.

🇧🇪Belgium rp7

rp7 changed the visibility of the branch 3474891-make-external-entities to hidden.

🇧🇪Belgium rp7

use Psr\Log\LoggerInterface;

It's already in the file, see line #18.

I think you can ask yourself: why is there even a getLogger() method, while the class already has a logger() method.

🇧🇪Belgium rp7

Not a real solution, but here's a quick workaround for whoever needs it (we aren't using the REST storage client, so the unique index is not really important for us anyway).

🇧🇪Belgium rp7

I upgraded my project (using several custom storage clients) from 2.x to 3.x. Here are some remarks:

It went better than expected. Nice work!
I did see some stuff in code I have doubts about/maybe would do differently. If it's OK by you, and once I find the time, I'll create a separate issue to list these & have a place to discuss them.

🇧🇪Belgium rp7

@guignonv
Thanks for the update. I have a project which makes heavily use of external entities (which is a reason I started the 2.x branch for). I'll free up some space to upgrade to v3 and report back on the things I had to change on my end.

My project is now in need for some extra features, such as Make external entities translatable Active and Allow to annotate without the use of inline entity form Active . I will be working on these the next few days (for v3). Could we get these on the roadmap for v3? They might require some (minor) breaking changes (not entirely sure yet) - so it would be nice to have them in before 3.x RC hits.

🇧🇪Belgium rp7

This is now in both the 2.x and 3.x branches. See Make external entities translatable Active for the next phase: making external entities translatable.

🇧🇪Belgium rp7

Taking a stab at this (for 2.x only at the moment). Will be report back as soon as I've made decent progress.

🇧🇪Belgium rp7

I've been heavily involved with the development of 2.x of this module (hence why I'm maintainer as well). Until a few months ago I wasn't aware a 3.x of this module was in the works. Is there an upgrade guide from 2.x to 3.x?

🇧🇪Belgium rp7

I had the exact same issue with the office hours form field displayed in a modal (loaded through AJAX). It looks like this is fixed in the DE-version of the module (most likely because of the work done in 📌 Refactor the JavaScript for the widget to use the once library Needs review ).

🇧🇪Belgium rp7

My question was primarily for EntityChangedInterface, which is provided by Drupal core and in use by some (sub)systems/contrib modules. I found out about this in a custom project of mine, where a piece of generic code didn't have the expected behavior in the case of external entities (e.g. if ($entity instance of EntityChangeInterface) { ... }).

I guess I can agree with the created-property, there doesn't seem to be an interface for this (while some core-entity types do implement it in their own interface, though).

If this is not desired, I guess I'll just have to extend the ExternalEntity class for my use-case.

🇧🇪Belgium rp7

Patched attached shows how the implementation would look like.
Is this something we would consider useful?

🇧🇪Belgium rp7

Updated patch so that the langcode field is also available on the form display (when creating/saving external entities).

🇧🇪Belgium rp7

@nord102 I'm not sure I agree with your reasoning.

It seems you might be mixing a taxonomy vocabulary and a collection of terms. A taxonomy vocabulary is more accurately described as a "type" of term rather than a collection of terms.

To illustrate this, consider the analogy with content types: just as we refer to a content type in the singular (e.g., "Article" for individual articles), it makes sense to refer to a taxonomy vocabulary in the singular (e.g., "Tag" for individual tags). Each term within a taxonomy vocabulary is a specific instance of that type. Therefore, the vocabulary itself should be named in the singular to reflect this relationship.

In your analogy, a view of nodes, such as "Articles," represents a collection of individual articles, which is why it is named in the plural. However, the taxonomy vocabulary serves a different role: it defines the type of terms that can be used, similar to how a content type defines the structure of individual pieces of content.

Using the singular form for taxonomy vocabularies maintains consistency and clarity, ensuring that we distinguish between the type (taxonomy vocabulary) and the instances of that type (terms within the vocabulary).

🇧🇪Belgium rp7

Original patch only applied to StringFilter. I expanded it so it also applies to NumericFilter.

This might be a valid reason to leave the option in FilterPluginBase (since it's being questioned in #4).
Upgrade path + test coverage is still TODO.

🇧🇪Belgium rp7

As I already mentioned by others, perhaps we should idd look into separating the 2 features.
Skipping unpublished content could use/benefit from the solution being worked on in Allow entities to be skipped programmatically Needs review .

🇧🇪Belgium rp7

Patch attached adds a SkipEntity-event (similar to the already present BuildHeader event) that allows entities to be skipped from link checker.

🇧🇪Belgium rp7

Patch attached is how a possible implementation could look like.

🇧🇪Belgium rp7

Experiencing the same issue on a project of ours. Running on 10.2.6. Patch fixes it, but I agree with #3 that this isn't really a long term solution.

🇧🇪Belgium rp7

Here's how the implementation could look like.

🇧🇪Belgium rp7

I was looking for a way to use feature flags within Drupal and stumbled upon this issue. I'm a bit surprised seeing modules being proposed, but I'm probably missing something obvious.

If we want the on/off state of feature flags to be deploy-able (I'm still trying to grasp exactly why - especially looking at tools like LaunchDarkly - in which feature flags are meant to be evaluated during runtime), why is a module preferred over just a true/false configuration setting? What benefit does it give us?

🇧🇪Belgium rp7

FWIW, the approach External Entities is taking (external entity types as config) was inspired by https://www.drupal.org/project/eck and (IMO) is certainly something that should be kept.

But I'm pretty sure @joachim is not asking to remove that part - rather support both scenario's (through config or through code).

🇧🇪Belgium rp7

@jsacksick
Might be a good idea to consider!
(not entirely sure how that would play with cacheability & invalidation, something to research)

🇧🇪Belgium rp7

The variations can be re-saved in the background (through a queue). So that removes the performance bottleneck.

🇧🇪Belgium rp7

The node_revision_delete module has no concept of content moderation which could potentially make it dangerous for moderated sites where forward revisions could be drafts and an historical revision could be your current published revision (see in getCandidatesNodes in node_revision_delete).

Could it be that you are talking about the first version of the node_revision_delete module? Because the 2.x version certainly is content moderation aware.

I can't find a getCandidatesNodes() method in the 2.x version of the module, so you are probably talking about the first version?

🇧🇪Belgium rp7

Updating tests. Expecting them to succeed now.

🇧🇪Belgium rp7

Made a few proposed changes + tried to get the tests green.
A test for an actual asset generation path should still be added, though.

🇧🇪Belgium rp7

Fixed deprecation notice: Deprecated function: Creation of dynamic property Drupal\\vbo_export\\Plugin\\Action\\VboExportDoc::$renderer is deprecated in Drupal\\vbo_export\\Plugin\\Action\\VboExportDoc->__construct().

🇧🇪Belgium rp7

Just as I created this ticket, it looks like this error is introduced by another patch we are using. Sorry for the noise. Closing this one.

Production build 0.71.5 2024