The entity_view function has been deprecated for some time now and only used in a couple of places. Let's go the home stretch, remove the final usages from core and trigger a deprecation warning when the method is called by implementing modules.

CommentFileSizeAuthor
#12 2974258-12.patch10.09 KBlegolasbo
#12 interdiff-10-12.txt909 byteslegolasbo
#10 2974258-10.patch9.2 KBlegolasbo
#4 2974258-4.patch9.93 KBlegolasbo
#4 interdiff-2-4.txt1.01 KBlegolasbo
remove-remaining-usages-of-entity-view.patch9.92 KBlegolasbo
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

legolasbo created an issue. See original summary.

legolasbo’s picture

legolasbo
he/him
Primary language Dutch
Location Middelburg
CreditAttribution: legolasbo as a volunteer commented
Issue summary: View changes

Status: Needs review » Needs work

The last submitted patch, remove-remaining-usages-of-entity-view.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

legolasbo’s picture

legolasbo
he/him
Primary language Dutch
Location Middelburg
CreditAttribution: legolasbo as a volunteer commented
Status: Needs work » Needs review
FileSize
interdiff-2-4.txt1.01 KB
2974258-4.patch9.93 KB

I think testbot is broken because

find /var/www/html/sites -type d -exec chmod 777 {} \;
14:04:17 PHP Warning:  file_put_contents(/var/lib/drupalci/workspace/jenkins-drupal_patches-59010/source/core/.env): failed to open stream: Permission denied in /opt/drupalci/testrunner/src/DrupalCI/Plugin/BuildTask/BuildStep/Testing/NightwatchJS.php on line 62
14:04:17 sudo BABEL_DISABLE_CACHE=1 -u www-data yarn --cwd /var/www/html/core test:nightwatch
14:04:23 yarn run v1.6.0
14:04:23 $ cross-env BABEL_ENV=development node -r dotenv-safe/config -r babel-register ./node_modules/.bin/nightwatch --config ./tests/Drupal/Nightwatch/nightwatch.conf.js
14:04:24 
14:04:24 /var/www/html/core/node_modules/dotenv-safe/index.js:31
14:04:24             throw new MissingEnvVarsError(allowEmptyValues, options.path || '.env', example, missing, dotenvResult.error);
14:04:24             ^
14:04:24 MissingEnvVarsError: The following variables were defined in .env.example but are not present in the environment:
14:04:24   DRUPAL_TEST_BASE_URL, DRUPAL_TEST_DB_URL, DRUPAL_TEST_WEBDRIVER_HOSTNAME, DRUPAL_TEST_WEBDRIVER_PORT, DRUPAL_TEST_CHROMEDRIVER_AUTOSTART, DRUPAL_NIGHTWATCH_OUTPUT, DRUPAL_NIGHTWATCH_IGNORE_DIRECTORIES
14:04:24 Make sure to add them to .env or directly to the environment.
14:04:24

doesn't seem to have anything to do with my patch and I've seen the exact same message show up on other patches. I did however fix the coding standards issue that was reported.

Status: Needs review » Needs work

The last submitted patch, 4: 2974258-4.patch, failed testing. View results

legolasbo’s picture

legolasbo
he/him
Primary language Dutch
Location Middelburg
CreditAttribution: legolasbo as a volunteer commented
Status: Needs work » Needs review

Back to needs review while retesting #4 now testbot is no longer broken.

Version: 8.6.x-dev » 8.7.x-dev

Drupal 8.6.0-alpha1 will be released the week of July 16, 2018, which means new developments and disruptive changes should now be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

joachim’s picture

joachim CreditAttribution: joachim as a volunteer commented
Status: Needs review » Reviewed & tested by the community

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 4: 2974258-4.patch, failed testing. View results

legolasbo’s picture

legolasbo
he/him
Primary language Dutch
Location Middelburg
CreditAttribution: legolasbo at Ordina Digital Services commented
Status: Needs work » Needs review
FileSize
2974258-10.patch9.2 KB

Patch rerolled

Status: Needs review » Needs work

The last submitted patch, 10: 2974258-10.patch, failed testing. View results

legolasbo’s picture

legolasbo
he/him
Primary language Dutch
Location Middelburg
CreditAttribution: legolasbo at Ordina Digital Services commented
Status: Needs work » Needs review
FileSize
interdiff-10-12.txt909 bytes
2974258-12.patch10.09 KB

Apparently the who's online block test was moved. glad we've got the deprecation warning :)

amateescu’s picture

amateescu CreditAttribution: amateescu for Pfizer, Inc. commented
Status: Needs review » Reviewed & tested by the community
Issue tags: +@deprecated

The patch looks great. We should open a similar issue for entity_view_multiple().

amateescu’s picture

amateescu CreditAttribution: amateescu for Pfizer, Inc. commented

Nevermind me, we already have #2974258: Remove remaining usages of entity_view :)

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch at Third and Grove commented 
+++ b/core/modules/node/node.module
@@ -812,7 +812,9 @@ function node_get_recent($number = 10) {
 function node_view(NodeInterface $node, $view_mode = 'full', $langcode = NULL) {

Is there an issue open to properly deprecate node_view()?

legolasbo’s picture

legolasbo
he/him
Primary language Dutch
Location Middelburg
CreditAttribution: legolasbo at Ordina Digital Services commented

I don't think there is @catch, and the same seems to be true for user_view() and taxonomy_term_view(). Those issues should be created, but that shouldn't block this patch from landing should it?

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan as a volunteer and at PreviousNext commented

Those issues should be created, but that shouldn't block this patch from landing should it?

Correct, they should be created, but are follow ups.

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan as a volunteer and at PreviousNext commented
Status: Reviewed & tested by the community » Needs work 
+++ b/core/includes/entity.inc
@@ -365,6 +365,9 @@ function entity_page_label(EntityInterface $entity, $langcode = NULL) {
+  $error_msg = __METHOD__ . ' is deprecated as of Drupal 8.0.x and will be removed before Drupal 9.0.0. Instead, use \Drupal::entityTypeManager()->getViewBuilder($entity->getEntityTypeId())->view($entity, $view_mode, $langcode);';
+  trigger_error($error_msg, E_USER_DEPRECATED);

We need a test that asserts this error is triggered (tagged with @Legacy)

voleger’s picture

voleger
he/his
Primary language Ukrainian
Location Ukraine, Rivne
CreditAttribution: voleger as a volunteer and at GOLEMS GABB, Drupal Ukraine Community for Drupal Ukraine Community, GOLEMS GABB commented
Related issues: +#2999721: [META] Deprecate the legacy include files before Drupal 9
andypost’s picture

andypost
he/him
Primary language Russian
CreditAttribution: andypost as a volunteer and at Skilld commented
Related issues: +#2974253: Remove remaining usages of entity_view and entity_view_multiple from core
andypost’s picture

andypost
he/him
Primary language Russian
CreditAttribution: andypost as a volunteer and at Skilld commented
Status: Needs work » Closed (duplicate)

Closing it as duplicate of #2974253: Remove remaining usages of entity_view and entity_view_multiple from core
Patch to deprecate remains and test it all in #2974253-13: Remove remaining usages of entity_view and entity_view_multiple from core