Problem/Motivation

FieldGroupRowsTest fails currently with PostgreSQL as database backend.

Proposed resolution

Identify and fix the failing tests.

Remaining tasks

Write patch.

User interface changes

None.

API changes

None.

CommentFileSizeAuthor
#5 test_group_rows_missing_order-5.patch1 KBbzrudi71
#1 test_group_rows_missing_order-1.patch1.58 KBbzrudi71
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

bzrudi71’s picture

bzrudi71 CreditAttribution: bzrudi71 commented
Status: Active » Needs review
FileSize
test_group_rows_missing_order-1.patch1.58 KB

And again, view with no kind of order. Passes PG, let's see if we get pass on MySQL too.

daffie’s picture

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

It all looks good to me.
I can confirm that the test fails for postgreSQL and with the patch the test passes for postgreSQL.
So for me it is RTBC.

Good work bzrudi71.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented 
  1. +++ b/core/modules/views/tests/modules/views_test_config/test_views/views.view.test_group_rows.yml
@@ -190,6 +186,28 @@ display:
+      sorts:
+        created:
+          id: created
+          table: node_field_data
+          field: created
+          relationship: none
+          group_type: group
+          admin_label: ''
+          order: ASC
+          exposed: false
+          expose:
+            label: ''
+          granularity: second
+          entity_type: node
+          entity_field: created
+          plugin_id: date

    Can't we just order by NID asc? This seems more simple to understand. 

  2. +++ b/core/modules/views/tests/modules/views_test_config/test_views/views.view.test_group_rows.yml
@@ -190,6 +186,28 @@ display:
+    cache_metadata:
+      contexts:
+        - node_view_grants
+        - user
+        - language
+      cacheable: false

@@ -198,3 +216,9 @@ display:
+    cache_metadata:
+      contexts:
+        - node_view_grants
+        - user
+        - language
+      cacheable: false

    I would just leave out the cache_metadata out of the view ... its not related

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick at Acquia commented
Status: Reviewed & tested by the community » Needs work
bzrudi71’s picture

bzrudi71 CreditAttribution: bzrudi71 commented
Status: Needs work » Needs review
FileSize
test_group_rows_missing_order-5.patch1 KB

Re #3.1

Can't we just order by NID asc? This seems more simple to understand.

That doesn't work as the field_group appears on the same nid - nid 1 in this case. No success when I tried this ;-)

Re #3.2
cache_metadata removed

daffie’s picture

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

It all looks good to me.
I can confirm that the test fails for postgreSQL and with the patch the test passes for postgreSQL.
All the requested changes are made or argued against.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Chapter Three commented
Status: Reviewed & tested by the community » Fixed

This issue addresses a major bug and is allowed per https://www.drupal.org/core/beta-changes. Committed cb230c2 and pushed to 8.0.x. Thanks!

  • alexpott committed cb230c2 on 8.0.x 
    Issue #2443695 by bzrudi71: PostgreSQL: Fix views\Tests\Handler\...

Status: Fixed » Closed (fixed)

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

mradcliffe’s picture

mradcliffe
he/him
Primary language English
CreditAttribution: mradcliffe commented

This seems to have failed again in the last 2 days.