ManyToOneHelper ignores group configuration for some cases

Updating the patch per xjm's comments from #115.

It looks like the reason for the fails is due to the change of default test themes from Classy to Stark ( https://www.drupal.org/project/drupal/issues/3248295 → ) which no longer inserts the view-content class the test originally relied on.

This changes the selector to CSS based on the views-element-container class which is added directly in Drupal\views\Element\View::preRenderViewElement rather than in a theme template.

The last submitted patch, 116: 2559961-116.patch, failed testing. View results →

Of course patch 116 "worked on my local machine" but fails on Drupal CI...

Trying out a different approach in the test to avoid counting elements on the page and instead checking for presence of the expected node titles in the test view output.

That's looking better, adding the interdiff for 105 and 118

Reading through earlier comments again, I agree with comment #95 🐛 ManyToOneHelper ignores group configuration for some cases Fixed in that the views_test_filter introduced in earlier patches isn't necessary--the newly added test uses an already existing view config test_filter_taxonomy_index_tid.

Updating the patch to remove the portion that adds the core/modules/views/tests/modules/views_test_filter module.

Attach the correct interdiff between patch #105 and #121 as txt instead of a patch file.

Thank you @angrytoast for patch from #121, works flawlessly on Drupal 9.4.8

This still needs extra test coverage so we can hit all the changes

Revisiting this one. After looking through history and specifically at comments #64 🐛 ManyToOneHelper ignores group configuration for some cases Fixed and #96 🐛 ManyToOneHelper ignores group configuration for some cases Fixed , adding patch to hit the additional test cases.

I'm not certain about modifying and saving the view repeatedly in the test. Thoughts?

Small update to the patch, missing a trailing comma which caused CI build fail.

OK. Builds are green, that's a good start. Marking as needs review again to get feedback on updated tests.

let's try with the latest

Created MR with latest patch #126 from the current dev branch 11.x

But for this scenario won't work properly ... taken from the related issue https://www.drupal.org/project/drupal/issues/1766338 🐛 Incorrect filter group OR behavior, LEFT JOIN changed to INNER JOIN Closed: duplicate
1. Create 2 taxonomies
and add some terms
2. Create 2 taxonomy term reference fields
And create different content
3. Create a view with 2 filters for those 2 fields
4. Play with AND and OR operators for those filters

So i "completed" the MR with the solution from there
+ Added the tests for this scenario.

And it passes

p.s. i have no idea why those 2 issues were separated.

@vasike are you mixing the two tickets?

Think the issue summary should be updated about proposed solution just for this ticket.

issue summary updated ... mention both fixes for this ticket and its MR.

Sorry if it's not super clear to me but:

Can the issue summary be updated with the steps to reproduce #130 helped but not entirely sure the bug to be on the look out for.

Also the proposed solution is missing

Brought this up in slack and appears it's one of those issues that didn't have enough test coverage. But not in scope of this ticket so opened 📌 Additional tests for ManyToOneHelper Active

updated the summary with more info about issue, step to reproduce and solution proposal.

I hope it's enough and it's not that technical ...

Thanks. Based on the proposed solution I think this is ready for committer review.

Thanks for everyone's patience here, with a useful issue summary and a great patch with an easy-to-understand fix and a comprehensive test case.

Committed and pushed aec9cf8ca1 to 11.x and 12f091439f to 10.1.x.

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