czigor → created an issue. See original summary → .
czigor → created an issue. See original summary → .
Added some CS and test fixes as well.
@milanbombschliip I've already started, thanks. And now the actual review part.
Finally hopefully I have some time to review this properly.
On the one hand https://github.com/ddev/ddev-drupal-contrib/blob/main/README.md says
Commit the changes in the .ddev folder after this plugin installs. This saves other users from having to install this integration.
However, https://www.drupal.org/project/search_api/issues/3520659#comment-16149749 ✨ Enabled local development env with DDEV Active says
Hi, DDEV maintainer here. I don't usually encourage committing .ddev in contexts like this.
I also feel bad about committing it, so let's remove all ide and ddev related additions from the MR.
Looks like a trivial fix to me.
Just in case someone finds this useful. I put the client id and secret into config (since that's easy to override by environment) and used the following hook:
/**
* Implements hook_ENTITY_TYPE_load().
*/
function mymodule_consumer_load($entities): void {
/** @var \Drupal\consumers\Entity\ConsumerInterface $consumer */
foreach ($entities as $consumer) {
if ($consumer->uuid() !== SomeClass::UUID_CONSTANT) {
continue;
}
$config = \Drupal::config('mymodule.settings');
if ($config->get('oauth_client_id')) {
$consumer->set('client_id', $config->get('oauth_client_id'));
}
if ($config->get('oauth_client_secret')) {
$hash = \Drupal::service('password')->hash(trim($config->get('oauth_client_secret')));
$consumer->set('secret', $hash);
}
}
}
Patch with enums.
Let's use class_exists()
instead.
This has been merged, thanks!
We could definitely add something like WHERE collection='entity.definitions.installed' AND name LIKE %.field_storage_definitions'
.
My bigger concern is if this is too database specific or not. We might want to do a SELECT, do the replacement in php and do a separate UPDATE instead.
To sum up our findings with @siegrist:
Since this commit commerce's BundleFieldDefinition
got deprecated in favor of the implementation of the entity module. This happened 7 years ago.
The issue is that the sites that existed before the deprecation might still have commerce's BundleFieldDefinition
serialized in their field storage definition. See e.g. select * from key_value where collection='entity.definitions.installed' and name='commerce_payment_method.entity_type';
So when updating to DC 3.x these definitions start referring to a nonexistent class.
This affects pretty much all payment method types, profile types and product types as well.
@jsacksick Would it make sense for you to have an update hook in commerce core 2.x that updates these field storage definitions to use the entity module implementation instead? (Once you update to 3.x it's too late.) I haven't tried but just saving the entity type might recreate the field storage definitions as well.
This one is fixing 09be1f63ff32bbc154f74c7367cb18023f814113 to keep the speed gain. Probably this is the better approach.
That's partially reverting 09be1f63ff32bbc154f74c7367cb18023f814113.
czigor → created an issue. See original summary → .
Patch #2 makes sense and fixes the issue for me as well.
I just checked the site again and now the bug is gone. I've rebooted my laptop since the last try.
What I've found so far is that something goes wrong during the plugin annotation parsing.
When language-url is not found the following happens:
- When running
drush cr
the annotation gets parsed inDrupal\Component\Annotation\Doctrine::parse()
. This time the "[#" string gets a correctT_ATTRIBUTE (351)
code which makes the annotation be parsed. Right after this the cache_discovery table contains the correct data for thelanguage_negotiation_plugins
cid. - After this the cache_discovery table is emptied due to clearing the caches).
- After this when I go to any page the plugin definitions are rebuilt. This time the "[#" string gets a
T_COMMENT (387)
code which makes the annotation be skipped. The cache entry will be an empty array.
Not sure yet why the difference is.
Experiencing this issue. I don't know what triggered it but now I can't get rid of it. The missing plugins it's complaining about is either language-url or block_page.
Adding D11 support.
They also offered that they could make name fields visible on the credit card form on their side. Because I have some concerns that the name we have in Drupal might not actually be the name on the credit card and it might be worse to have the wrong name as opposed to no name?
Yes, these two names might not necessarily coincide although at least in Hungary we have some rule that they should. (I.e. I cannot pay with my personal card if I ask for an invoice for some company.)
@Berdir Thanks for the review! Fixed the issues. Since VISA will stop working without 3DS, I think some boilerplate 3DS handling has its place in contrib.
Adding
$this->userStorage->resetCache();
before
$account = $this->userStorage->load(1);
$account->init = $account->mail = $account_values['mail'];
in \Drupal\Core\Installer\Form\SiteConfigureForm::submitForm()
makes the issue go away.
Wondering whether this is a Drupal core issue.
We see this issue as well when running our tests on a site and it breaks our CI. However, I was not able to reproduce this on a minimal drupal install (where only commerce_payment_example was enabled).
@omarlopesino Do you know what the other modules or config is required to see this?
Let's remove the custom order item fields requirement.
czigor → created an issue. See original summary → .
The deprecation happens only from Drupal 10.3 so the MR fails.
czigor → created an issue.
czigor → created an issue.