Problem/Motivation

The test class in core/modules/path/tests/src/Functional/PathTaxonomyTermTest.php uses a regular database query instead of using an entity query.

Proposed resolution

Replace the regular database query with an entity query.

Remaining tasks

TBD

User interface changes

None

API changes

None

Data model changes

None

Release notes snippet

TBD

CommentFileSizeAuthor
#14 interdiff-7-14.txt610 bytesHardik_Patel_12
#14 3151959-14.patch1.16 KBHardik_Patel_12
#7 interdiff-2-7.txt424 bytesHardik_Patel_12
#7 3151959-7.patch1.14 KBHardik_Patel_12
#2 3151959-2.patch1019 bytesdaffie
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

daffie created an issue. See original summary.

daffie’s picture

daffie CreditAttribution: daffie commented
Status: Active » Needs review
FileSize
3151959-2.patch1019 bytes

Patch changes the db query to an entity query.

Status: Needs review » Needs work

The last submitted patch, 2: 3151959-2.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

munish.kumar’s picture

munish.kumar CreditAttribution: munish.kumar as a volunteer and at Srijan | A Material+ Company for Drupal India Association commented

Looks like a random test failure, so re-queued.

munish.kumar’s picture

munish.kumar CreditAttribution: munish.kumar as a volunteer and at Srijan | A Material+ Company for Drupal India Association commented
Status: Needs work » Needs review
Hardik_Patel_12’s picture

Hardik_Patel_12 CreditAttribution: Hardik_Patel_12 at QED42 commented
Status: Needs review » Reviewed & tested by the community

Verified this patch on local. Looks good to me.

Hardik_Patel_12’s picture

Hardik_Patel_12 CreditAttribution: Hardik_Patel_12 at QED42 for Drupal India Association commented
Status: Reviewed & tested by the community » Needs review
FileSize
3151959-7.patch1.14 KB
interdiff-2-7.txt424 bytes

Just small thing remaining, removing unused statement use Drupal\Core\Database\Database; , otherwise all is good to me.

daffie’s picture

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

@Hardik_Patel_12: Good find!

Back to RTBC.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 7: 3151959-7.patch, failed testing. View results

munish.kumar’s picture

munish.kumar CreditAttribution: munish.kumar as a volunteer and at Srijan | A Material+ Company for Drupal India Association commented
Status: Needs work » Reviewed & tested by the community

All the test cases are passed now, and the last patch only contains the small change so moving back to RTBC

pratik_kamble’s picture

pratik_kamble
Location Pune, India
CreditAttribution: pratik_kamble at QED42 commented
Issue tags: +Bug Smash Initiative
pratik_kamble’s picture

pratik_kamble
Location Pune, India
CreditAttribution: pratik_kamble at QED42 commented
Issue tags: -Bug Smash Initiative
catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch at Third and Grove commented
Status: Reviewed & tested by the community » Needs work 
+++ b/core/modules/path/tests/src/Functional/PathTaxonomyTermTest.php
@@ -52,7 +51,11 @@ public function testTermAlias() {
     $this->drupalPostForm('admin/structure/taxonomy/manage/' . $vocabulary->id() . '/add', $edit, t('Save'));
-    $tid = Database::getConnection()->query("SELECT tid FROM {taxonomy_term_field_data} WHERE name = :name AND default_langcode = 1", [':name' => $edit['name[0][value]']])->fetchField();
+    $tids = \Drupal::entityQuery('taxonomy_term')
+      ->condition('name', $edit['name[0][value]'])
+      ->condition('default_langcode', 1)
+      ->execute();

This needs ->accessCheck(FALSE)

Hardik_Patel_12’s picture

Hardik_Patel_12 CreditAttribution: Hardik_Patel_12 at QED42 for Drupal India Association commented
Status: Needs work » Needs review
FileSize
3151959-14.patch1.16 KB
interdiff-7-14.txt610 bytes

Adding the requested ->accessCheck(FALSE) to EntityQuery.

daffie’s picture

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

Change looks like what @catch requested.
Back to RTBC.

  • catch committed d8d9481 on 9.1.x 
    Issue #3151959 by Hardik_Patel_12, daffie: Replace the database query...
catch’s picture

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

Committed d8d9481 and pushed to 9.1.x. Thanks!

Status: Fixed » Closed (fixed)

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