Remove deprecated EntityDefinitionUpdateManagerInterface::applyUpdates() from hook_install and hook_uninstall [#3038746]

EntityDefinitionUpdateManagerInterface::applyUpdates() is deprecated in 8.7.0 and will be removed before Drupal 9.0.0.
Use \Drupal\Core\Entity\EntityDefinitionUpdateManagerInterface::getChangeList() and execute each entity type and field storage update manually instead.

Core issue: #2976035: Entity type CRUD operations must use the last installed entity type and field storage definitions
Change record: https://www.drupal.org/node/3034742

The trigger_error deprecation warning was added to 8.7 Core/Entity/EntityDefinitionUpdateManager.php applyUpdates() on 8th March

Our deprecation test patch has the detailed results https://www.drupal.org/pift-ci-job/1222503
A summary of the errors can be seen in the TravisCI Scheduler test log:

Remaining deprecation notices (29)
  29x: EntityDefinitionUpdateManagerInterface::applyUpdates() is deprecated in 8.7.0 and will be removed before Drupal 9.0.0. Use \Drupal\Core\Entity\EntityDefinitionUpdateManagerInterface::getChangeList() and execute each entity type and field storage update manually instead. See https://www.drupal.org/node/3034742.
    1x in SchedulerPastDatesTest::testSchedulerPastDates from Drupal\Tests\scheduler\Functional
    1x in SchedulerPermissionsTest::testUserPermissions from Drupal\Tests\scheduler\Functional
    1x in SchedulerNonEnabledTypeTest::testNonEnabledNodeType from Drupal\Tests\scheduler\Functional
    1x in SchedulerNodeAccessTest::testNodeAccess from Drupal\Tests\scheduler\Functional
    1x in SchedulerMultilingualTest::testPublishingTranslations from Drupal\Tests\scheduler\Functional
    1x in SchedulerRequiredTest::testRequiredScheduling from Drupal\Tests\scheduler\Functional
    1x in SchedulerRevisioningTest::testRevisioning from Drupal\Tests\scheduler\Functional
    1x in SchedulerTokenReplaceTest::testSchedulerTokenReplacement from Drupal\Tests\scheduler\Functional
    1x in SchedulerValidationTest::testValidationDuringEdit from Drupal\Tests\scheduler\Functional
    1x in SchedulerStatusReportTest::testStatusReport from Drupal\Tests\scheduler\Functional
    1x in SchedulerScheduledContentListAccessTest::testViewScheduledContentOverview from Drupal\Tests\scheduler\Functional
    1x in SchedulerRevisioningTest::testAlterCreationDate from Drupal\Tests\scheduler\Functional
    1x in SchedulerScheduledContentListAccessTest::testViewScheduledContentUser from Drupal\Tests\scheduler\Functional
    1x in SchedulerMetaInformationTest::testMetaInformation from Drupal\Tests\scheduler\Functional
    1x in SchedulerLightweightCronTest::testLightweightCronRun from Drupal\Tests\scheduler\Functional
    1x in SchedulerApiTest::testNidList from Drupal\Tests\scheduler\Functional
    1x in SchedulerApiTest::testNidListAlter from Drupal\Tests\scheduler\Functional
    1x in SchedulerApiTest::testApiNodeAction from Drupal\Tests\scheduler\Functional
    1x in SchedulerApiTest::testAllowedUnpublishing from Drupal\Tests\scheduler\Functional
    1x in SchedulerAdminSettingsTest::testAdminSettings from Drupal\Tests\scheduler\Functional
    1x in SchedulerApiTest::testAllowedPublishing from Drupal\Tests\scheduler\Functional
    1x in SchedulerBasicTest::testPublishingAndUnpublishing from Drupal\Tests\scheduler\Functional
    1x in SchedulerDefaultTimeTest::testDefaultTime from Drupal\Tests\scheduler\Functional
    1x in SchedulerFieldsDisplayTest::testManageFormDisplay from Drupal\Tests\scheduler\Functional
    1x in SchedulerFieldsDisplayTest::testDisabledFields from Drupal\Tests\scheduler\Functional
    1x in SchedulerFieldsDisplayTest::testVerticalTabOrFieldset from Drupal\Tests\scheduler\Functional
    1x in SchedulerDeleteNodeTest::testDeleteNodeWithPastDates from Drupal\Tests\scheduler\Functional
    1x in SchedulerDeleteNodeTest::testDeleteNodeWhenSchedulingIsRequired from Drupal\Tests\scheduler\Functional
    1x in SchedulerLightweightCronTest::testLightweightCronSettingsForm from Drupal\Tests\scheduler\Functional

Comment	File	Size	Author
#3	3038746-3.remove_applyUpdates.patch	987 bytes	jonathan1055
#3	8.x-1.x: PHP 5.6 & MySQL 5.5, D8.8 35 pass

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

9 March 2019 at 09:21

jonathan1055 created an issue. See original summary.

Comment #2

jonathan1055 CreditAttribution: jonathan1055 as a volunteer commented 9 March 2019 at 14:44

Issue summary:

View changes

All of the warnings (fairly obviously) are produced from code in scheduler.install

/**
 * Implements hook_install().
 */
function scheduler_install() {
  // Add our base fields to the schema.
  \Drupal::service('entity.definition_update_manager')->applyUpdates();
  // Set cron access key value, as this is now required in SchedulerCronForm.
  $config = \Drupal::service('config.factory')->getEditable('scheduler.settings');
  $config->set('lightweight_cron_access_key', substr(md5(rand()), 0, 20))
    ->save();
}

/**
 * Implements hook_uninstall().
 */
function scheduler_uninstall() {
  // Remove our base fields from the schema.
  \Drupal::service('entity.definition_update_manager')->applyUpdates();
  // Delete the scheduled content view.
  \Drupal::configFactory()->getEditable('views.view.scheduler_scheduled_content')->delete();
}

Comment #3

jonathan1055 CreditAttribution: jonathan1055 as a volunteer commented 9 March 2019 at 19:13

Status:

Active

» Needs review

File	Size
3038746-3.remove_applyUpdates.patch	987 bytes
8.x-1.x: PHP 5.6 & MySQL 5.5, D8.8 35 pass

Do we actually need to explicitly add our base fields during install and remove then on uninstall? I know there had been problems in the early days of Drupal 8, but this may be better now.

The passing tests wont necessarily prove that it is OK to remove ->applyUpdates() but it's a good starting point.

Comment #4

jonathan1055 CreditAttribution: jonathan1055 as a volunteer commented 10 March 2019 at 11:44

According to the first answer on my drupal.stackexchange.com question it is not necessary to call applyUpdates() in hook_install() and hook_uninstall() which is what I was hoping for (and also expecting)

However, when checking that the fields have been removed I ran into this old problem #2924256: Entity displays need to depend on the module that provides base fields again, where our base fields are still left in the content type form display, which just confuses the task of verifying that the fields have been removed.

Comment #5

jonathan1055 CreditAttribution: jonathan1055 as a volunteer commented 12 March 2019 at 13:09

The automated tests pass, and I have manually checked this too. All seems to be OK during install and uninstall so I'm going to commit this.

Comment #6

12 March 2019 at 13:10

jonathan1055 committed 8f8120a on 8.x-1.x

Issue #3038746 by jonathan1055: Remove deprecated...

Comment #7

jonathan1055 CreditAttribution: jonathan1055 as a volunteer commented 12 March 2019 at 13:12

Title:	Replace deprecated EntityDefinitionUpdateManagerInterface::applyUpdates() with ::getChangeList()	» Remove deprecated EntityDefinitionUpdateManagerInterface::applyUpdates() from hook_install and hook_uninstall
Status:	Needs review	» Fixed

Fixed