Problem/Motivation

When we have multiple PB instances and if we select any category from Filter by category dropdown then the values are selected for the first instance in all the cases.

Steps to reproduce

Visit project-browser/project_browser_test_mock?instances=4 and select the categories for any of the instance except the first one and then you might notice that the categories are selected for the first instance and it's result are updated.

Proposed resolution

Issue fork project_browser-3511942

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

utkarsh_33 created an issue. See original summary.

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented
Issue tags: +Needs tests

We'll need a test for this (a new method of MultipleInstancesTest should suffice), and it's definitely a beta blocker.

narendrar made their first commit to this issue’s fork.

narendrar opened merge request !782

narendrar’s picture

narendrar
He/Him
Primary language English
Location Jaipur, India
commented
Status: Active » Needs review

I couldn't reproduce this issue in tests, although it was reproducible locally. This fix resolved the issue, and I updated the existing test to cover the scenario of selecting the second instance first.

utkarsh_33’s picture

utkarsh_33 commented

I can confirm that the issue is solved with this Fix.

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented
Status: Needs review » Needs work

Fixed merge conflicts, and suggested an alternate (simpler) approach here that might work.

The bigger problem, though, is that the test passes locally even against 2.0.x...so it's not catching the bug. :( The test needs to fail on 2.0.x without the patch.

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented

Another question: how is it that the existing test didn't catch this bug?

At the bottom of the test method in 2.0.x, we are confirming that adding an additional category to the second instance produces a different list of results than the first instance. Why didn't this fail in 2.0.x?

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented

I did a little bit of investigating and discovered that the reason the tests aren't catching the bug is because...even in 2.0.x, the test behaves correctly!

And yet, if I put a sleep(30) at the top of the test, and then click around manually in Chrome...I am seeing the buggy behavior. In other words, this is a case where the human interaction of clicking on the checkbox causes the bug, but Mink's interaction with the checkbox does not.

That leads me to believe that the problem is something to do with, maybe, the onChange function in MultipleChoiceFilter.svelte. If it's not that, another suspicious section of the code is the really dodgy DOM traversal/queries happening in showHideFilter to deal with focusing -- it's entirely possible that they're somehow looking at the wrong part of the DOM tree.

I think we need to take a few steps back here and start with a test that unambiguously fails against 2.0.x. We're not going to have any other way to know for sure if we've fixed this bug.

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented

I can also confirm that if I pause the test at the beginning with a sleep(30) and click around, my proposed fix appears to prevent the bug.

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented

Ensuring credit is correct, as @narendra and @utkarsh_33 worked to reproduce this in a test while I was asleep.

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented
Status: Needs work » Needs review
Issue tags: -Needs tests

Figured out the problem -- clicking the label causes the problem; clicking the checkbox does not. The tests normally click the checkbox.

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented

Confirmed that tests catch the bug; the test-only job fails as expected.

chrisfromredfin made their first commit to this issue’s fork.

chrisfromredfin’s picture

chrisfromredfin
he/him/his
Primary language English
Location Portland, Maine
commented
Status: Needs review » Fixed

tests are passing, PHPStan issue present in 2.0.x-HEAD

Status: Fixed » Closed (fixed)

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