🇨🇦Canada @colan

Thanks for the help.

The first problem is that Search API extends ConfigEntityStorage, and we extend ContentEntityStorage.

I'm going to look into whether it makes sense to implement the method, which core calls from here. It's defined at SqlEntityStorageInterface::getTableMapping(), and there's an example at SqlContentEntityStorage::getTableMapping().

colan → created an issue.

colan → created an issue.

colan → created an issue.

Updated title so I stop getting it confused with ✨ Translation support in vertical data aggregation Active .

I'm also wondering if it has anything to do with this TODO.

This is now a blocker for us so I need to start working on it. Can you provide any tips, advice or steps to approach the problem to help me get moving on it? Anything you can offer would be helpful.

Composer-friendly patch.

Thanks!

Why are we muting a deprecation notice instead of using the newer API? Seems like we're treating a symptom here instead of the root cause. Or am I missing something?

I think it is an edge case that someone would intentionally enter in the same email address using different cases.

True.

The issue is not about modifying user's data, its about validation.

Also true.

What hasn't been mentioned here is that the part before the "@" could be case sensitive if mail servers are configured that way (see discussion), even though it's silly. So leaving this closed for that reason isn't crazy.

However, if I were to post a patch here, I'd be using strcasecmp(), not strtolower(). 😉

Here's a Composer-friendly patch for 6.2.x, which assumes that #3452986-18: Update jquery.intl-tel-input to v23.0.10 and add option for i18n → has already been applied.

Here's a Composer-friendly patch for 6.2.x.

Accidentally changed some things; just setting them back.

I thought this was a support request, and someone provided an answer in #5. So it seems resolved to me.

(I don't think this is the same ID problem we had in v3, 🐛 Newly created entity "cannot have a URI as it does not have an ID" Active . If it is, someone from v2 can reopen.)

Resolve merge conflicts, but tests are failing. :(

Resolved merge conflict. Problem was that we're hacking on the file modules/webform_toggles/js/webform_toggles.element.js that's no longer there due to 📌 Deprecate the use of drgullin/icheck Active . I simply removed the file. If folks are interested in making this change in Webform Deprecated → , they can dig it out of the commit.

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

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

Thanks for offering! Will steal it from you if we need it sooner.

Added the Views issue to the next beta, but not sure if we should cut another release before that. There's been lots of activity lately.

Works for me. Merci à tous!

@guignonv: What do you think? Tests are still passing, so that's good.

If someone makes me a co-maintainer, I can help with this stuff.

This is really a documentation issue as it's not obvious how to do this. Here's what we need to do:

• There's no example in the README with a contextual filter in the JSON File config field. Let's add one with the info from #3170134-2: Contextual Filter Path Value to JSON Path → .
• Change the default branch from 8.x-1.x to 2.0.x (because clicking on Source code from the project page still take you to 8.x-1.x).
• Add a documentation link from the project page (so that it shows up on the right sidebar). Should link to https://git.drupalcode.org/project/views_json_source/-/blob/2.0.x/README...
• Convert README.txt to README.md (optional, but would be formatted nicely in GitLab). If doing this, also update the link above.

colan → created an issue.

colan → created an issue.

I think we can merge this after tests are passing.

@guignonv: So the merge request (MR) is good so far? No major changes required?

@sugen: I would uncomment it and keep it it in. If it does nothing for now, that's fine, but at least we won't have to remember to add it back in later if/when we actually do need it.

Amazing work; thanks!

I took a look at the code and it makes sense. We'll try it here, and get back to you. (I just turned the branch into an MR for easier viewing.)

This one's back on we've confirmed that it happens even after we're returning the IDs and getting 200 OK.

It's with a Single Storage Client: POSTing/creating is successful, but then there's a WSOD on the front end. Coming at it from another way, GET/fetching data fails (and results in the same WSOD).

If we comment out these lines, it works fine:

So it's coming from ExternalEntityForm->save(), the two toLink() calls.

Specially, that's ✨ Allow mapping of file URI to a field Active (with the funky markup rendering).

Well, it's better than getting an error, right?

This same error message is discussed in a core issue ( 🐛 Drupal\file\IconMimeTypes doesn't handle NULL mimetypes Active ), but that seems to more about actually missing mime types. This one here appears to be caused by the way EE is handling it.

Or the DB setting is wrong.

I'm mentoring Andrew, and I worked on this with him.

Thanks!

Marking this as a duplicate of 🌱 Save between steps Active . This one's older, but is on an old branch.

From #3178227: Save between each step → , which I marked as a duplicate:

We used a hook_form_alter to add this:

    $form['actions']['next']['#validate'][] = '::validateForm';
    $form['actions']['next']['#validate'][] = '::submitForm';
    $form['actions']['next']['#validate'][] = '::save';
    $form['actions']['back_button']['#validate'][] = '::validateForm';
    $form['actions']['back_button']['#validate'][] = '::submitForm';
    $form['actions']['back_button']['#validate'][] = '::save';

(I agree with 🐛 Validation handler of the next button breaks other form validation Fixed , it feels strange to have this in #validate and not in #submit.)

But it would be nice to have a supported option directly in the module.

This is now in the new release.

beta4 → is out.

Looks like if you merge it from here, it squashes it. I don't like that. Will do it from GitLab next time.

I did the following:

• Placed the token service in StorageClientBase so all clients can use it.
• Fixed the namespace path to the token service.
• Removed the Token module as dependency as this is Core functionality.

@guignonv: What do you think? Can we merge?

Would also be nice to cut a new beta after we get this in; I can do this if you'd like.

Just a one-liner so far, but we should probably use dependency injection, injecting the `token` service into the RestClient.

Needs to be a single patch, not several, or it won't apply.

Here's a Composer-friendly patch.

Yes, it works. Just tested it!

@filsterjisah & others: Besides fixing merge conflicts and getting tests to pass, I also refactored RefreshAccessTokenSubscriber so that the code is cleaner. Kindly review & try it? If all if well, we can RTBC this so it gets in soon.

In that case, the RTBC was really for the non-v3 MR, not your patch, which needs review.

However, I just reviewed your code and it looks good. Also, we tested it as part of testing the other issue so RTBC actually does make sense now so leaving status as-is.

Thanks for the update!

Okay, so we need to figure out how to incorporate both approaches without collision.

Looks like this is now incorporated into 💬 Store and Use Refresh Token on Expiry Active so assuming that gets in, let's give credit to the folks here too. For now, I think we should direct our efforts over there.

Pipeline is failing due to failed tests. Might just be that we need to add the new stuff to the schema:

1) Drupal\Tests\openid_connect\Functional\UserPermissionTest::testDisconnectFormSubmissionsForOthers with data set "administer openid connect clients permission" ('administer openid connect clients', false)
Drupal\Core\Config\Schema\SchemaIncompleteException: Schema errors for openid_connect.client.test_oidc_label with the following errors: openid_connect.client.test_oidc_label:settings.auto_refresh_expired_tokens missing schema
/builds/issue/openid_connect-3327440/web/core/lib/Drupal/Core/Config/Development/ConfigSchemaChecker.php:98
/builds/issue/openid_connect-3327440/vendor/symfony/event-dispatcher/EventDispatcher.php:206
/builds/issue/openid_connect-3327440/vendor/symfony/event-dispatcher/EventDispatcher.php:56
/builds/issue/openid_connect-3327440/web/core/lib/Drupal/Core/Config/Config.php:230
/builds/issue/openid_connect-3327440/web/core/lib/Drupal/Core/Config/Entity/ConfigEntityStorage.php:260
/builds/issue/openid_connect-3327440/web/core/lib/Drupal/Core/Entity/EntityStorageBase.php:486
/builds/issue/openid_connect-3327440/web/core/lib/Drupal/Core/Config/Entity/ConfigEntityStorage.php:239
/builds/issue/openid_connect-3327440/web/core/lib/Drupal/Core/Entity/EntityBase.php:354
/builds/issue/openid_connect-3327440/web/core/lib/Drupal/Core/Config/Entity/ConfigEntityBase.php:617
/builds/issue/openid_connect-3327440/tests/src/Functional/UserPermissionTest.php:97
/builds/issue/openid_connect-3327440/tests/src/Functional/UserPermissionTest.php:114

@rp7: Would you kindly let us know what's left to do here? Thanks.

Adding ✨ Translation support in vertical data aggregation Active as related.

I think most of that token functionality is in core now, except for the token browser (for that you need the Token module → ), but what I have in my head could be out-of-date. Actually this seems true as of Drupal 7! See https://www.drupal.org/documentation/modules/token → .

So no, we don't need to add that dependency.

That field is working fine, except that it needs to support replaceable Drupal tokens (as I mentioned above). I've opened a separate v3 ticket for this: ✨ Authentication Custom Header should support Drupal token replacements Active

We started working on a patch for this, but ran into some other priorities. Will have to come back to this later if nobody else does.

colan → created an issue.

v2 is no longer recommended; see 📌 [META] Roadmap for v3.0 Active .

If nobody cares, we can mark it unsupported? I left it supported because there are recent commits to the branch.

Marked v2 as supported again as I noticed some recent commits by other folks.

Here was our conversation via e-mail exchange:

On Sat, Feb 8, 2025 at 10:35 PM I wrote:

Can we set v3 to Recommended and stop recommending v2? Or do you want to keep v2 as Recommended?

On 2025-02-10 06:02, Attiks wrote:

v3 is fine by me.

On Mon, Feb 10, 2025 at 4:10 PM I wrote:

Great. Are you still maintaining v2, or should we mark it unsupported?

On 2025-02-10 10:23, Attiks wrote:

Feel free to mark it unsupported

So I was able to:

• Mark v2 as unsupported and marked v3 as Recommended.

I could not do these, because I don't have enough permissions (@guignonv: Can you do these as you're a Maintainer?):

• Change the default branch to v3 because I'm only a Developer.
• Deprecate the v2 Views module; I'm not a maintainer there at all.
• Enabled automated testing (don't see the tab).

The remote payload that was coming back was missing IDs so there was nothing to map with the Drupal entity IDs. So this error actually makes sense. Sorry for the noise!

colan → created an issue.

Thanks! What are your thoughts on v2?

I used his contact form yesterday to ask about co-maintaining (and also pointed him to this thread), but haven't heard back yet.

Updated status of Views support.

+++ b/src/Plugin/ExternalEntities/StorageClient/Rest.php
@@ -180,41 +170,57 @@ class Rest extends ExternalEntityStorageClientBase implements PluginFormInterfac
+      $sequence[$exploded[0]] = $value;

Would it make sense to add a Token::replacePlain() in here to expand tokens in values?

In our case, users will each have an ID that they pass in for authentication & authorization so it'll be something like [User:remoteID].

Thanks for the update! You've done an impressive amount of work here, and thanks for updating the Views doc page.

For my current project, we're starting to put together a proof of concept (PoC) for relying heavily on this module. So assuming that goes well, we should be able to help with stabilizing the issues that we're interested in, which includes:

• ✨ Make external entities translatable Needs work
• #3261988: Allow optional separate endpoint for query request on REST client plugin →
• ✨ Adding fields to add values in headers Needs review
• ✨ Support external entities related custom cache tag Needs review
• ✨ Authentication plugin system for use with the Rest external storage client Active
• 📌 Move automatic generation of path aliases to the pathauto submodule Active
• 🐛 Improve UX for deleting annotations/external entities Needs work
• ✨ what if entity is not found Needs review
• #3136194: Arguments in API endpoint path →
• ✨ Support Drupal's Entity Reference field type concept on external data source. Active
• 📌 Add basic test coverage Active

For now though, I think we should do a few things quickly; I can do these if you make me a maintainer:

• Deprecate the v2 Views module (which is no longer needed) in favour of the main module. (I'd also need maintainer access for this one.)
• Switch the release recommendation on the main project page from v2 to v3.
• Enabled automated testing in the project as I mentioned at 📌 Add basic test coverage Active .

There's more happening in ✨ what if entity is not found Needs review . Maybe mark this as a duplicate of that one?

Is this still relevant?

For now, let's at least enable automated testing in the project settings so that we can see if patches apply. (There are a lot of v2 patches/MRs that may or may not work in v3.)

This sounds overly complicated. What about simply having a config tab for the entity that allows you do choose which fields are entity references? By default, it could translate the remote ID value into the Drupal entity ID, and use that to make the reference?

For more complicated values in the reference field, it could allow for hook implementations that provide a translation in code.

@burnellw_cit: Kindly provide an interdiff? If you can't, what changed? What's different about your patch?

I didn't run a `diff`, but at a quick glance, the patch and the MR look the same. Let's work in the MR though? Patches can just be for Composer.

Looks like ✨ Make external entities language aware Fixed is in.

Follow-up is ✨ Make external entities translatable Needs work .

Is this roadmap still relevant, or do we only need to talk about v3 now?

Hi! I'm glad to see this is still active after we talked about it 10 years ago 📌 Make Views use SqlEntityStorageInterface Needs work . Would you be able to provide the status of Views integration?

• 📌 Views integration Needs review looks done
• The docs → say it's not possible, but use the Search API
• There's a separate contrib module for it →

My client is telling me that a CC didn't help; needed the patch.

Back to needing a reroll...

If anyone's getting this after a cache clear, please holler.