guignonv → credited 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.
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).
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).
Re-rolled against 2.0.0
+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));
}
}
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?
Sorry for the late reply, but it looks like it works. Thanks!
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;
}
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.
@guignonv
I think that was idd something that creeped into the MR when resolving conflicts. Reverted that change.
Fork works now. I'm hiding the patch. Latest changes can be seen in the MR.
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 ).
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.
@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?
Thx! Maybe a new issue would be useful for such a clean-up.
I think you forgot to merge this, though (:
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.
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.
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).
rp7 → created an issue.
I upgraded my project (using several custom storage clients) from 2.x to 3.x. Here are some remarks:
- The update hooks transforming the config to be compatible with 3.x worked flawlessly!
- Most of the things I had to do is already documented in https://git.drupalcode.org/project/external_entities/-/blob/3.0.x/DEVELO... .
- In order to alter some filters, I had some custom hooks I invoked from my custom storage clients. I was able to replace these successfully with the TRANSLITERATE_DRUPAL_FILTERS event.
- I had some custom logic in my storage clients related to sorting parameters sent to the external source. I couldn't find a way to replace these, so I created ✨ Allow to alter sorts through event subscriber Needs review .
- I faced some other issues, for which I created several tickets. ( 🐛 ID is not assigned to newly saved external entities Needs review , 🐛 JsonPath::getMappedSourceFieldName() incompatible with various JSONPath notations Needs review , 🐛 Incompatibility with Monolog Needs review )
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.
rp7 → created an issue.
rp7 → created an issue. See original summary → .
@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.
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.
Taking a stab at this (for 2.x only at the moment). Will be report back as soon as I've made decent progress.
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?
Of course. Forked + created an MR.
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 ).
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.
Patched attached shows how the implementation would look like.
Is this something we would consider useful?
Updated patch so that the langcode field is also available on the form display (when creating/saving external entities).
rp7 → created an issue. See original summary → .
@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).
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.
mstrelan → credited 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
.
Patch attached adds a SkipEntity-event (similar to the already present BuildHeader event) that allows entities to be skipped from link checker.
Patch attached is how a possible implementation could look like.
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.
Here's how the implementation could look like.
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?
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).
@jsacksick
Might be a good idea to consider!
(not entirely sure how that would play with cacheability & invalidation, something to research)
The variations can be re-saved in the background (through a queue). So that removes the performance bottleneck.
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?
Updating tests. Expecting them to succeed now.
Updating tests.
Updating tests.
Made a few proposed changes + tried to get the tests green.
A test for an actual asset generation path should still be added, though.
MR opened + patch attached.
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().
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.