Replace all db calls to block_content table with Entity APIs

As I understand it, this test does not verify what is declared. $enabled_block_content->id() is not equal $disabled_block_content->id() and for enabled block the query will be return 1 anyway.

$rows = db_query('SELECT * FROM {block_content_field_data} WHERE id = :id', [':id' => $enabled_block_content->id()])->fetchAll();an
$this->assertEqual(1, count($rows)); 

And the last test checks correctly but it does not passed. If 4 years ago the current test was correct, we would see the moment when changes were made, after which the test would cease to pass, as is happening now. So it seems we catch the bug of https://www.drupal.org/project/drupal/issues/1916790 and we have to wait until this new test will be passed after the issue bug will be fixed.

The test should be converted to using table mapping to get table name in this kind of testing to keep logic same

This issue should be closed.
Calls to db_query function are already replaced and pushed in 8.8 branch.
See https://www.drupal.org/project/drupal/issues/2875394#comment-13123165

-duplicated comment-

@mondrake so sorry! :)

This issue is out of time. Core doesn't have any call to db_query except at the tests that cover it and functions. So I think that this issue can be closed

Exactly this is a reason why we need a reroll on this issue. db_query functions were just replaced by the query method calls on the database connection instance. So the scope of this issue is still the same.

I've rerolled the patch from #4.

Below are my observations towards the implementation.

$rows = Database::getConnection()->query('SELECT * FROM {block_content_field_data} WHERE id = :id', [':id' => $enabled_block_content->id()])->fetchAll();

- Earlier code was fetching from block_content_field_data table with condition of block id which will always return value 1 since we are passing id of block created.

 $query = \Drupal::entityQuery('block_content')->condition('langcode','en','=')->count();

- Above code is added in the patch which will generate a count which shows the block having language English
- In the code hereafter, we have

    $disabled_block_content = $this->createBlockContent(FALSE, $bundle->id(), LanguageInterface::LANGCODE_NOT_SPECIFIED);

- Language Code when added for $disabled_block_content should not have langcode as english
- Adding langcode as en by default will always give the count to be 2
- hence i have added an addiational parameter langcode to the createBlockContent()
so that the first block will create a count of block with en as the language and second block created will have language not specified, which eventually passes the test.

The patch looks good. I have just 1 minor point:

+++ b/core/modules/block_content/tests/src/Functional/BlockContentTranslationUITest.php
@@ -91,17 +93,18 @@ public function getTranslatorPermissions() {
+  protected function createBlockContent($title = FALSE, $bundle = FALSE, $langcode = 'en') {

@@ -170,14 +173,13 @@ public function testDisabledBundle() {
+    $query = \Drupal::entityQuery('block_content')->condition('langcode','en','=')->count();
...
+    $disabled_block_content = $this->createBlockContent(FALSE, $bundle->id(), LanguageInterface::LANGCODE_NOT_SPECIFIED);

For all 3 places: use the same value. Either "en" or LanguageInterface::LANGCODE_NOT_SPECIFIED.

@daffie, will change it to en everywhere

As nobody seams to understand what the test Drupal\Tests\block_content\Functional\BlockContentTranslationUITest::testDisabledBundle() is supposed to test. Can we just remove the test?

@Neslle: I think you can also delete the helper method createBlockContent(), when you delete testDisabledBundle().

+++ b/core/modules/block_content/tests/src/Functional/BlockContentTranslationUITest.php
@@ -6,6 +6,8 @@
+use Drupal\Core\Language\LanguageInterface;
+

These lines will then also not be needed. The same for use statements for Drupal\block_content\Entity\BlockContent and Drupal\Core\Database\Database.

This patch has to go first to 9.1.

Patch for 9.1.x

As nobody seems to understand what the test Drupal\Tests\block_content\Functional\BlockContentTranslationUITest::testDisabledBundle() is supposed to test. Lets remove the test.

All code changes look good.
All database queries of the block_content module that can be changed, have been changed to an entity query.
For me it is RTBC.

@Neslee: Thank you for all of your patches.

It looks like testing block content with revisions disabled, but it looks strange

Could we double check why Drupal\Tests\block_content\Functional\BlockContentTranslationUITest::testDisabledBundle() was added via git blame? Agreed it's an odd looking test that doesn't seem to match its documentation.

The removed test Drupal\Tests\block_content\Functional\BlockContentTranslationUITest::testDisabledBundle() was added in #1871772: Convert custom blocks to content entities. I think we can assume that all custom blocks are now content entities.

Committed 219df57 and pushed to 9.1.x, cherry-picked to 9.0.x. Thanks!