Skip to content
This repository was archived by the owner on Oct 18, 2023. It is now read-only.

Consolidated Drupal 9 Compatibility#164

Open
japerry wants to merge 14 commits intoacquia:8.x-1.xfrom
japerry:acm_d9
Open

Consolidated Drupal 9 Compatibility#164
japerry wants to merge 14 commits intoacquia:8.x-1.xfrom
japerry:acm_d9

Conversation

@japerry
Copy link
Contributor

@japerry japerry commented Mar 5, 2020

No description provided.

Comment on lines +117 to +142
$storage_definition = BaseFieldDefinition::create('boolean')
->setLabel(t('Status'))
->setDescription(t('Whether the SKU is available or not.'))
->setDefaultValue(TRUE);

\Drupal::entityDefinitionUpdateManager()
->installFieldStorageDefinition('status', 'acm_sku', 'acm_sku', $storage_definition);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do these functions throw exceptions? If yes, please catch and log as before. If no, do they write to the log?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think so, this is using the same pattern that Node uses in core, see the following:
https://git.drupalcode.org/project/drupal/-/blob/8.8.x/core/modules/node/node.install#L169

$config->setData($existing_fields)->save();

if ($apply_updates) {
$this->entityDefinitionUpdateManager->applyUpdates();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand why this code is removed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automatic entity updates no longer exist in Drupal 8.7+, it is handled in the entity manager for save, therefore the existing code changing the entity should suffice.

*/
public function createProductOptionWrapper($langcode, $option_id, $option_value, $attribute_id, $attribute_code, $weight) {
return $this->createProductOption();
return $this->createProductOption($langcode, $option_id, $option_value, $attribute_id, $attribute_code, $weight);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a bug fix, yes?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that was taken from the merge done from the earlier PR. If you want this moved out we can do it, but its pretty minor so I just kept it in.

@japerry
Copy link
Contributor Author

japerry commented Apr 16, 2020

This should be reopned.

@japerry japerry reopened this Apr 16, 2020
@japerry
Copy link
Contributor Author

japerry commented May 15, 2020

The latest fix puts us in a good position to release for Drupal 9. There are some outstanding deprecations around REQUEST_TIME -- which I'll see if we need to resolve, but as of May 15 2020 -- Drupal core itself has pushed some of these deprecations to 9.1. One of the problems is the service will not work with our unit tests, which will require some more refactoring.

There is also an issue with facets, but since it doesn't have a release, we cannot test our plugin. I don't think thats a major issue at the moment.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants