We are currently loading or creating GroupContent entities in special ways outside of the storage. This should be moved to the storage so the CRUD logic is all in one logical place.Here's a

CommentFileSizeAuthor
#7 interdiff-6-7.txt2.6 KBkristiaanvandeneynde
#7 group-2867710-7.patch19.48 KBkristiaanvandeneynde
#3 group-2867710-3.patch19.07 KBkristiaanvandeneynde
#2 group-2867710-2-poc.patch10.15 KBkristiaanvandeneynde
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

kristiaanvandeneynde created an issue. See original summary.

kristiaanvandeneynde’s picture

kristiaanvandeneynde
Primary language Dutch
Location Antwerp, Belgium
CreditAttribution: kristiaanvandeneynde at Deeson commented
Issue summary: View changes
Status: Active » Needs review
FileSize
group-2867710-2-poc.patch10.15 KB

Here's a proof of concept. Going to write some tests for it.

kristiaanvandeneynde’s picture

kristiaanvandeneynde
Primary language Dutch
Location Antwerp, Belgium
CreditAttribution: kristiaanvandeneynde at Deeson commented
FileSize
group-2867710-3.patch19.07 KB

And now with tests for the entire storage handler.

Status: Needs review » Needs work

The last submitted patch, 3: group-2867710-3.patch, failed testing.

kristiaanvandeneynde’s picture

kristiaanvandeneynde
Primary language Dutch
Location Antwerp, Belgium
CreditAttribution: kristiaanvandeneynde at Deeson commented

So all of the sudden the test are tripping over warnings thrown by #2744069: views_query_views_alter() does not handle IN queries?

kristiaanvandeneynde’s picture

kristiaanvandeneynde
Primary language Dutch
Location Antwerp, Belgium
CreditAttribution: kristiaanvandeneynde at Deeson commented

Okay so adding the 2nd group type in the group_test_config module broke core again. With one group type, there is only one group content type during the Views tests and then this piece of code kicks in in core:

// FROM: \Drupal\views\Plugin\views\join\JoinPluginBase::buildJoin()

// Convert a single-valued array of values to the single-value case,
// and transform from IN() notation to = notation
if (is_array($info['value']) && count($info['value']) == 1) {
  $info['value'] = array_shift($info['value']);
}

Which is why it didn't use to break and all of the sudden it does: It never was an IN-query because of this optimization and now it is, meaning we run into the bug in #2744069: views_query_views_alter() does not handle IN queries

So if I were to change the test to dynamically generate a 2nd group type, we should be fine. But that is another stopgap for a core bug, so next up is fixing core...

kristiaanvandeneynde’s picture

kristiaanvandeneynde
Primary language Dutch
Location Antwerp, Belgium
CreditAttribution: kristiaanvandeneynde at Deeson commented
Status: Needs work » Needs review
FileSize
group-2867710-7.patch19.48 KB
interdiff-6-7.txt2.6 KB

This should work.

Status: Needs review » Needs work

The last submitted patch, 7: group-2867710-7.patch, failed testing.

kristiaanvandeneynde’s picture

kristiaanvandeneynde
Primary language Dutch
Location Antwerp, Belgium
CreditAttribution: kristiaanvandeneynde at Deeson commented
Status: Needs work » Needs review

Test is green and "failed testing"? Weird stuff.

kristiaanvandeneynde’s picture

kristiaanvandeneynde
Primary language Dutch
Location Antwerp, Belgium
CreditAttribution: kristiaanvandeneynde at Deeson commented
Status: Needs review » Fixed

Committed with the @todo's regarding the bug in core.

Status: Fixed » Closed (fixed)

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