Parent Issue

#2359437: [meta] Remove entity_load* family of functions usage from the code base

Problem/Motivation

There are cases where entities of the tour entity type are retrieved using the functional loader, entity_load()

Proposed resolution

Replace calls to entity_load('tour', ...) with Tour::load*()

CommentFileSizeAuthor
#3 2723631.patch1.52 KBpbuyle
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

valthebald created an issue. See original summary.

pbuyle’s picture

pbuyle CreditAttribution: pbuyle at Floe design + technologies commented

I'm at DrupalCon NOLA and looking into this.

pbuyle’s picture

pbuyle CreditAttribution: pbuyle at Floe design + technologies commented
Status: Active » Needs review
FileSize
2723631.patch1.52 KB

I found two occurrences of entity_load(_multiple(_by_properties)?)?\('tour'. The attached patch removes them.

Status: Needs review » Needs work

The last submitted patch, 3: 2723631.patch, failed testing.

pbuyle’s picture

pbuyle CreditAttribution: pbuyle at Floe design + technologies commented
Status: Needs work » Needs review

Tests are passing now.

Mile23’s picture

Mile23
Primary language English
Location Seattle, WA
CreditAttribution: Mile23 as a volunteer commented

OK, not sure what's up with the tests, re-running.

John Cook’s picture

John Cook CreditAttribution: John Cook commented
Issue summary: View changes

Updated summary.

John Cook’s picture

John Cook CreditAttribution: John Cook commented
Status: Needs review » Reviewed & tested by the community

After applying the patch from #3, there are no more occurences of entity_load*\('tour'

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 3: 2723631.patch, failed testing.

valthebald’s picture

valthebald
Location Sofia
CreditAttribution: valthebald as a volunteer commented

It's better not to set the status to RTBC until testbot gives its ok

John Cook’s picture

John Cook CreditAttribution: John Cook commented
Status: Needs work » Needs review

@valthebald the test had passed when I set it to RTBC. I think the testbot is in a bad mood ;)

Resetting to Needs Review to run the tests.

Mile23’s picture

Mile23
Primary language English
Location Seattle, WA
CreditAttribution: Mile23 as a volunteer commented

The way to run the tests now is to click 'add tests' in the test results.

I just ran them which is why it failed.

If you click on the result, it shows you a log of what failed. In this case, I think it might be unrelated, since that migrate test has been flaking out in general.

Status: Needs review » Needs work

The last submitted patch, 3: 2723631.patch, failed testing.

Mile23’s picture

Mile23
Primary language English
Location Seattle, WA
CreditAttribution: Mile23 as a volunteer commented
Status: Needs work » Needs review

It looks like the problem test was fixed: #2725755: Fatal error: Cannot redeclare class Drupal\Tests\migrate\Unit\TestMigration

Re-running the test.

Status: Needs review » Needs work

The last submitted patch, 3: 2723631.patch, failed testing.

John Cook’s picture

John Cook CreditAttribution: John Cook commented
Status: Needs work » Reviewed & tested by the community

As the testbot has given the green light, setting back to RTBC for #3.

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch at Third and Grove commented
Status: Reviewed & tested by the community » Fixed

Committed 4bc14c7 and pushed to 8.2.x. Thanks!

  • catch committed 4bc14c7 on 8.2.x 
    Issue #2723631 by pbuyle, John Cook, valthebald, Mile23: Remove...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.